merge: T74 turn-flow polish + addressee service
This commit is contained in:
@@ -0,0 +1,99 @@
|
||||
"""Addressee classifier service tests (T74.1).
|
||||
|
||||
Covers :func:`chat.services.addressee.detect_addressee`:
|
||||
|
||||
- Classifier picks the guest -> ``addressee_id == guest_id``.
|
||||
- Classifier picks the host -> ``addressee_id == host_id``.
|
||||
- Classifier flakes (3 bad-JSON responses, exhausting the built-in
|
||||
retry budget in :func:`chat.llm.classify.classify`) -> fallback to
|
||||
the host with ``reason="fallback"``.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
|
||||
import pytest
|
||||
|
||||
from chat.llm.mock import MockLLMClient
|
||||
from chat.services.addressee import AddresseeDecision, detect_addressee
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_classifier_picks_guest():
|
||||
"""Classifier returns the guest id verbatim — caller propagates it."""
|
||||
canned = [
|
||||
json.dumps(
|
||||
{
|
||||
"addressee_id": "bot_b",
|
||||
"confidence": "high",
|
||||
"reason": "user named BotB",
|
||||
}
|
||||
)
|
||||
]
|
||||
client = MockLLMClient(canned=canned)
|
||||
|
||||
result = await detect_addressee(
|
||||
client,
|
||||
classifier_model="test-model",
|
||||
user_prose="BotB, what do you think?",
|
||||
host_id="bot_a",
|
||||
host_name="BotA",
|
||||
guest_id="bot_b",
|
||||
guest_name="BotB",
|
||||
)
|
||||
|
||||
assert isinstance(result, AddresseeDecision)
|
||||
assert result.addressee_id == "bot_b"
|
||||
assert result.confidence == "high"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_classifier_picks_host():
|
||||
"""Classifier returns the host id — caller propagates it."""
|
||||
canned = [
|
||||
json.dumps(
|
||||
{
|
||||
"addressee_id": "bot_a",
|
||||
"confidence": "medium",
|
||||
"reason": "narration aimed at host",
|
||||
}
|
||||
)
|
||||
]
|
||||
client = MockLLMClient(canned=canned)
|
||||
|
||||
result = await detect_addressee(
|
||||
client,
|
||||
classifier_model="test-model",
|
||||
user_prose="I lean back and stretch.",
|
||||
host_id="bot_a",
|
||||
host_name="BotA",
|
||||
guest_id="bot_b",
|
||||
guest_name="BotB",
|
||||
)
|
||||
|
||||
assert result.addressee_id == "bot_a"
|
||||
assert result.confidence == "medium"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_classifier_failure_falls_back_to_host():
|
||||
"""Three bad-JSON responses exhaust the retry budget and the
|
||||
classifier-failure fallback returns ``host_id`` with
|
||||
``reason="fallback"``."""
|
||||
canned = ["not json", "still not json", "garbage"]
|
||||
client = MockLLMClient(canned=canned)
|
||||
|
||||
result = await detect_addressee(
|
||||
client,
|
||||
classifier_model="test-model",
|
||||
user_prose="anything",
|
||||
host_id="bot_a",
|
||||
host_name="BotA",
|
||||
guest_id="bot_b",
|
||||
guest_name="BotB",
|
||||
)
|
||||
|
||||
assert result.addressee_id == "bot_a"
|
||||
assert result.reason == "fallback"
|
||||
assert result.confidence == "low"
|
||||
+236
-26
@@ -405,14 +405,15 @@ def test_multi_bot_turn_no_interjection(app_state_setup, tmp_path):
|
||||
1 user_turn + 1 assistant_turn + 6 *post-turn* edge_updates + 2
|
||||
memory_written events. Single turn_html broadcast.
|
||||
|
||||
Canned queue (8 calls):
|
||||
Canned queue (11 calls):
|
||||
1. parse_turn
|
||||
2. narrative stream (primary, addressee = host because the prose
|
||||
2. detect_addressee (T74.1) -> host
|
||||
3. narrative stream (primary, addressee = host because the prose
|
||||
doesn't name the guest)
|
||||
3-8. 6 state-update calls (one per directed pair across {you,
|
||||
4-9. 6 state-update calls (one per directed pair across {you,
|
||||
bot_a, bot_b})
|
||||
9. detect_interjection -> should_interject=False
|
||||
10. detect_scene_close -> should_close=False
|
||||
10. detect_interjection -> should_interject=False
|
||||
11. detect_scene_close -> should_close=False
|
||||
"""
|
||||
_seed_chat_with_guest(tmp_path / "test.db")
|
||||
canned_parse = json.dumps(
|
||||
@@ -420,6 +421,9 @@ def test_multi_bot_turn_no_interjection(app_state_setup, tmp_path):
|
||||
)
|
||||
canned = [
|
||||
canned_parse,
|
||||
json.dumps(
|
||||
{"addressee_id": "bot_a", "confidence": "medium", "reason": "host"}
|
||||
),
|
||||
"Greetings.",
|
||||
_zero_state(), _zero_state(), _zero_state(),
|
||||
_zero_state(), _zero_state(), _zero_state(),
|
||||
@@ -474,14 +478,15 @@ def test_multi_bot_turn_with_interjection(app_state_setup, tmp_path):
|
||||
1 user_turn + 2 assistant_turns + (6 + 6) post-turn edge_updates +
|
||||
4 memory_written events.
|
||||
|
||||
Canned queue (16 calls):
|
||||
Canned queue (17 calls):
|
||||
1. parse_turn
|
||||
2. narrative stream (primary)
|
||||
3-8. 6 state-update calls (post-primary)
|
||||
9. detect_interjection -> should_interject=True
|
||||
10. narrative stream (interjection)
|
||||
11-16. 6 state-update calls (post-interjection)
|
||||
17. detect_scene_close -> should_close=False
|
||||
2. detect_addressee (T74.1) -> host
|
||||
3. narrative stream (primary)
|
||||
4-9. 6 state-update calls (post-primary)
|
||||
10. detect_interjection -> should_interject=True
|
||||
11. narrative stream (interjection)
|
||||
12-17. 6 state-update calls (post-interjection)
|
||||
18. detect_scene_close -> should_close=False
|
||||
"""
|
||||
_seed_chat_with_guest(tmp_path / "test.db")
|
||||
canned_parse = json.dumps(
|
||||
@@ -489,6 +494,9 @@ def test_multi_bot_turn_with_interjection(app_state_setup, tmp_path):
|
||||
)
|
||||
canned = [
|
||||
canned_parse,
|
||||
json.dumps(
|
||||
{"addressee_id": "bot_a", "confidence": "medium", "reason": "host"}
|
||||
),
|
||||
"Primary beat.",
|
||||
_zero_state(), _zero_state(), _zero_state(),
|
||||
_zero_state(), _zero_state(), _zero_state(),
|
||||
@@ -555,14 +563,15 @@ def test_multi_bot_turn_scene_close_writes_per_pov_summaries(
|
||||
rewrites fire for both bots (memory.pov_summary changes for each).
|
||||
Interjection short-circuits at False so the queue stays compact.
|
||||
|
||||
Canned queue (12 calls):
|
||||
Canned queue (13 calls):
|
||||
1. parse_turn
|
||||
2. narrative stream (primary)
|
||||
3-8. 6 state-update calls
|
||||
9. detect_interjection -> False (no follow-on stream)
|
||||
10. detect_scene_close -> True
|
||||
11. apply_scene_close_summary host POV
|
||||
12. apply_scene_close_summary guest POV
|
||||
2. detect_addressee (T74.1) -> host
|
||||
3. narrative stream (primary)
|
||||
4-9. 6 state-update calls
|
||||
10. detect_interjection -> False (no follow-on stream)
|
||||
11. detect_scene_close -> True
|
||||
12. apply_scene_close_summary host POV
|
||||
13. apply_scene_close_summary guest POV
|
||||
"""
|
||||
_seed_chat_with_guest(tmp_path / "test.db")
|
||||
canned_parse = json.dumps(
|
||||
@@ -588,6 +597,9 @@ def test_multi_bot_turn_scene_close_writes_per_pov_summaries(
|
||||
)
|
||||
canned = [
|
||||
canned_parse,
|
||||
json.dumps(
|
||||
{"addressee_id": "bot_a", "confidence": "medium", "reason": "host"}
|
||||
),
|
||||
"Goodnight.",
|
||||
_zero_state(), _zero_state(), _zero_state(),
|
||||
_zero_state(), _zero_state(), _zero_state(),
|
||||
@@ -639,12 +651,20 @@ def test_multi_bot_turn_scene_close_writes_per_pov_summaries(
|
||||
|
||||
|
||||
def test_addressee_detection_routes_to_named_bot(app_state_setup, tmp_path):
|
||||
"""Prose that names the guest by name routes the primary turn to the
|
||||
guest. Interjection (when fired) makes the host the silent witness
|
||||
and the second assistant_turn carries the host as speaker.
|
||||
"""T74.1: the multi-entity addressee call goes through the classifier;
|
||||
when the classifier returns the guest, the primary turn routes there.
|
||||
Interjection (when fired) makes the host the silent witness and the
|
||||
second assistant_turn carries the host as speaker.
|
||||
|
||||
Canned queue: same shape as the with-interjection test (16 calls)
|
||||
plus the trailing scene_close decision.
|
||||
Canned queue (with classifier-led addressee = guest):
|
||||
1. parse_turn
|
||||
2. detect_addressee -> bot_b (the guest)
|
||||
3. narrative stream (primary, addressee = guest)
|
||||
4-9. 6 state-update calls
|
||||
10. detect_interjection -> True
|
||||
11. interjection narrative stream
|
||||
12-17. 6 state-update calls (post-interjection)
|
||||
18. detect_scene_close -> False
|
||||
"""
|
||||
_seed_chat_with_guest(tmp_path / "test.db")
|
||||
canned_parse = json.dumps(
|
||||
@@ -652,6 +672,13 @@ def test_addressee_detection_routes_to_named_bot(app_state_setup, tmp_path):
|
||||
)
|
||||
canned = [
|
||||
canned_parse,
|
||||
json.dumps(
|
||||
{
|
||||
"addressee_id": "bot_b",
|
||||
"confidence": "high",
|
||||
"reason": "user named BotB",
|
||||
}
|
||||
),
|
||||
"BotB pondering.",
|
||||
_zero_state(), _zero_state(), _zero_state(),
|
||||
_zero_state(), _zero_state(), _zero_state(),
|
||||
@@ -680,9 +707,192 @@ def test_addressee_detection_routes_to_named_bot(app_state_setup, tmp_path):
|
||||
primary_payload = json.loads(rows[0][0])
|
||||
interjection_payload = json.loads(rows[1][0])
|
||||
|
||||
# Primary speaker is the guest because the prose names BotB and not
|
||||
# BotA (case-insensitive whole-word match).
|
||||
# Primary speaker is the guest because the addressee classifier
|
||||
# picked bot_b for the prose ("BotB, what do you think?").
|
||||
assert primary_payload["speaker_id"] == "bot_b"
|
||||
# Interjection follow-on goes to the silent witness — the host.
|
||||
assert interjection_payload["speaker_id"] == "bot_a"
|
||||
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.
|
||||
|
||||
Capture enqueued ``SignificanceJob``s by replacing the background
|
||||
worker's ``enqueue`` method with a list-append. Without T74.2, the
|
||||
interjection memory would never be scored — only the primary's
|
||||
enqueue would land. We therefore expect TWO jobs after a turn that
|
||||
has both a primary and an interjection beat: one for the primary
|
||||
memory, one for the interjection memory.
|
||||
"""
|
||||
_seed_chat_with_guest(tmp_path / "test.db")
|
||||
canned_parse = json.dumps(
|
||||
{"segments": [{"kind": "dialogue", "text": "tell me"}]}
|
||||
)
|
||||
canned = [
|
||||
canned_parse,
|
||||
json.dumps(
|
||||
{"addressee_id": "bot_a", "confidence": "medium", "reason": "host"}
|
||||
),
|
||||
"Primary beat.",
|
||||
_zero_state(), _zero_state(), _zero_state(),
|
||||
_zero_state(), _zero_state(), _zero_state(),
|
||||
json.dumps({"should_interject": True, "reason": "jealous"}),
|
||||
"Interjection beat!",
|
||||
_zero_state(), _zero_state(), _zero_state(),
|
||||
_zero_state(), _zero_state(), _zero_state(),
|
||||
json.dumps({"should_close": False, "reason": "no signal"}),
|
||||
]
|
||||
_override_llm(canned)
|
||||
|
||||
captured_jobs: list = []
|
||||
worker = app.state.background_worker
|
||||
# Re-enable enqueue capture even though the worker's loop is disabled
|
||||
# — we want to count enqueues without the loop running classifier work.
|
||||
worker.enabled = True
|
||||
original_enqueue = worker.enqueue
|
||||
worker.enqueue = captured_jobs.append # type: ignore[assignment]
|
||||
|
||||
try:
|
||||
response = app_state_setup.post(
|
||||
"/chats/chat_bot_a/turns", data={"prose": "tell me"}
|
||||
)
|
||||
assert response.status_code == 204
|
||||
finally:
|
||||
worker.enqueue = original_enqueue # type: ignore[assignment]
|
||||
worker.enabled = False
|
||||
app.dependency_overrides.clear()
|
||||
|
||||
# Expect 2 enqueues: 1 for the primary memory + 1 for the
|
||||
# interjection memory.
|
||||
assert len(captured_jobs) == 2
|
||||
|
||||
# Both jobs should reference distinct memory ids — the primary's
|
||||
# host-POV memory and the interjection's host-POV memory.
|
||||
memory_ids = [job.memory_id for job in captured_jobs]
|
||||
assert len(set(memory_ids)) == 2
|
||||
# The two narrative texts should be the two streamed beats.
|
||||
narrative_texts = sorted(job.narrative_text for job in captured_jobs)
|
||||
assert narrative_texts == ["Interjection beat!", "Primary beat."]
|
||||
|
||||
Reference in New Issue
Block a user