diff --git a/chat/web/turns.py b/chat/web/turns.py index ab1308c..48882f0 100644 --- a/chat/web/turns.py +++ b/chat/web/turns.py @@ -668,6 +668,15 @@ async def post_turn( # close in the same chat) — we have nothing to close. T13 (kickoff) # is the only scene-opener path in v1; Phase 2-3 will handle # automatic re-opening with the next container. + # + # T74.3: this branch deliberately runs even when ``cancelled`` is + # True. Close detection consumes only the user's prose (which is + # fully appended to the event_log BEFORE streaming starts) and the + # current container name; it does NOT consume the bot's output. + # A user who types "we're done here, fade out" and then hits Stop + # mid-stream still meant to close the scene — the cancelled bot + # beat doesn't invalidate that intent. Pinned by + # test_cancelled_turn_still_closes_scene_when_user_prose_signals_close. if scene is not None and prose.strip(): container = None if scene.get("container_id") is not None: diff --git a/tests/test_turn_flow.py b/tests/test_turn_flow.py index 281d123..a30ec24 100644 --- a/tests/test_turn_flow.py +++ b/tests/test_turn_flow.py @@ -715,6 +715,127 @@ def test_addressee_detection_routes_to_named_bot(app_state_setup, tmp_path): assert interjection_payload["interjection_of"] == "bot_b" +def test_cancelled_turn_still_closes_scene_when_user_prose_signals_close( + app_state_setup, tmp_path +): + """T74.3 regression: a cancelled primary stream still triggers scene + close when the user prose carries a hard close signal. + + Rationale (also documented in turns.py near the close-detection + branch): close detection only consumes the user's prose, which is + fully appended to the event_log BEFORE streaming starts. The + cancelled bot beat doesn't invalidate the user's intent to close. + + Implementation: install a MockLLMClient whose ``stream`` raises + CancelledError on the first iteration. The classifier calls (parse, + addressee, scene_close, per-POV summaries) are still served from + the canned queue. The post_turn route ultimately re-raises + CancelledError after recording the partial — TestClient surfaces + that as an exception, so we drive the request inside ``with + pytest.raises``. Despite the exception, the scene_closed event + must land in the event_log. + """ + from typing import AsyncIterator, Sequence + + _seed_chat_with_guest(tmp_path / "test.db") + canned_parse = json.dumps( + {"segments": [{"kind": "narration", "text": "we are done here, fade out"}]} + ) + pov_payload = json.dumps( + { + "summary": "BotA noticed the day winding down.", + "knowledge_facts": [], + "relationship_summary": "warmer", + } + ) + pov_payload_guest = json.dumps( + { + "summary": "BotB watched the scene close.", + "knowledge_facts": [], + "relationship_summary": "warmer", + } + ) + # Canned queue: parse + addressee + 6 state-updates + + # scene_close=True + 2 per-POV summaries. NO interjection slot + # because the cancel path short-circuits the interjection branch. + canned = [ + canned_parse, + json.dumps( + {"addressee_id": "bot_a", "confidence": "medium", "reason": "host"} + ), + # NOTE: no narrative slot — the stream is hijacked below to + # raise CancelledError on first iteration; it never pulls a + # canned response. + _zero_state(), _zero_state(), _zero_state(), + _zero_state(), _zero_state(), _zero_state(), + json.dumps({"should_close": True, "reason": "fade out signaled"}), + pov_payload, + pov_payload_guest, + ] + + class _CancelOnStreamMock: + """Mock LLM client that serves ``generate`` from a canned queue + and raises CancelledError on the FIRST iteration of ``stream``. + + Mirrors :class:`chat.llm.mock.MockLLMClient` for ``generate`` but + diverges on ``stream`` to simulate a mid-stream cancel. + """ + + def __init__(self, canned: list[str]) -> None: + self._canned = list(canned) + + async def generate( + self, messages: Sequence, *, model: str, **params + ) -> str: + return self._canned.pop(0) + + async def stream( + self, messages: Sequence, *, model: str, **params + ) -> AsyncIterator[str]: + # Yield a CancelledError on first iteration to simulate the + # /turns/cancel route firing mid-stream. + raise asyncio.CancelledError + yield # pragma: no cover — keeps this an async generator. + + from chat.web.kickoff import get_llm_client + + mock = _CancelOnStreamMock(canned=list(canned)) + app.dependency_overrides[get_llm_client] = lambda: mock + + try: + # FastAPI/Starlette handles the re-raised CancelledError as an + # internal failure — TestClient surfaces it as a 500 response. + # We don't assert on the status here; the regression is whether + # the scene_closed event still landed in the event_log. + try: + app_state_setup.post( + "/chats/chat_bot_a/turns", + data={"prose": "we are done here, fade out"}, + ) + except BaseException: + # Some Starlette/asyncio versions propagate the + # CancelledError out of the test client; that's fine — the + # partial-record + scene-close still ran before the raise. + pass + finally: + app.dependency_overrides.clear() + + with open_db(tmp_path / "test.db") as conn: + scene_close_count = conn.execute( + "SELECT COUNT(*) FROM event_log WHERE kind = 'scene_closed'" + ).fetchone()[0] + assistant_payload = conn.execute( + "SELECT payload_json FROM event_log " + "WHERE kind = 'assistant_turn' ORDER BY id" + ).fetchall() + + # Scene close lands despite the cancel. + assert scene_close_count == 1 + # The cancelled assistant_turn was still recorded (truncated=True). + assert len(assistant_payload) == 1 + assert json.loads(assistant_payload[0][0])["truncated"] is True + + def test_interjection_enqueues_significance_job(app_state_setup, tmp_path): """T74.2: when an interjection fires, the interjection memory is enqueued for significance scoring just like the primary memory.