diff --git a/chat/services/memory_write.py b/chat/services/memory_write.py index 12eed5d..d60c3d9 100644 --- a/chat/services/memory_write.py +++ b/chat/services/memory_write.py @@ -22,62 +22,6 @@ from sqlite3 import Connection from chat.eventlog.log import append_and_apply -def record_turn_memory( - conn: Connection, - *, - chat_id: str, - host_bot_id: str, - narrative_text: str, - scene_id: int | None = None, - chat_clock_at: str | None = None, - source: str = "direct", - significance: int = 1, -) -> tuple[int, int | None]: - """Append a ``memory_written`` event for the host bot's POV of this turn. - - Uses :func:`chat.eventlog.log.append_and_apply` (not raw - :func:`append_event`) so the new memory row is projected immediately - without re-running prior non-idempotent handlers (e.g. ``edge_update`` - deltas). - - Returns ``(event_id, memory_id)``. ``event_id`` is the row id of the - just-appended ``memory_written`` event in ``event_log``. ``memory_id`` - is the autoincrement PK of the corresponding ``memories`` row — these - are *different* numbers (event_log and memories use independent - rowid sequences) so callers needing to update significance or pin - state must use ``memory_id``. Falls back to ``None`` if the projected - row can't be located, which shouldn't happen but keeps the return - shape stable. - """ - payload: dict = { - "owner_id": host_bot_id, - "chat_id": chat_id, - "pov_summary": narrative_text, - "witness_you": 1, - "witness_host": 1, - "witness_guest": 0, - "source": source, - "reliability": 1.0, - "significance": significance, - "pinned": 0, - "auto_pinned": 0, - } - if scene_id is not None: - payload["scene_id"] = scene_id - if chat_clock_at is not None: - payload["chat_clock_at"] = chat_clock_at - - event_id = append_and_apply(conn, kind="memory_written", payload=payload) - row = conn.execute( - "SELECT id FROM memories " - "WHERE owner_id = ? AND chat_id = ? " - "ORDER BY id DESC LIMIT 1", - (host_bot_id, chat_id), - ).fetchone() - memory_id = row[0] if row else None - return event_id, memory_id - - def _write_one_memory( conn: Connection, *, diff --git a/chat/services/regenerate.py b/chat/services/regenerate.py index b2aba9a..0678a76 100644 --- a/chat/services/regenerate.py +++ b/chat/services/regenerate.py @@ -182,9 +182,13 @@ async def regenerate_assistant_turn( (chat_id, original_assistant_event_id), ).fetchall() if unrolled_lifecycle: + # T90.2: phrased as "at-or-after turn " rather than "from + # superseded turn" because regenerating an OLDER turn lists + # intervening-turn transitions that legitimately stand on their + # own — those weren't authored by the superseded turn itself. _log.warning( - "regenerate_assistant_turn: %d lifecycle transition(s) from " - "superseded turn %s are NOT being rolled back (Phase 4 " + "regenerate_assistant_turn: %d lifecycle transition(s) " + "at-or-after turn %s are NOT being rolled back (Phase 4 " "follow-up). Affected event ids: %s", len(unrolled_lifecycle), original_assistant_event_id, diff --git a/chat/services/turn_common.py b/chat/services/turn_common.py index e246314..3c63420 100644 --- a/chat/services/turn_common.py +++ b/chat/services/turn_common.py @@ -54,14 +54,21 @@ def read_recent_dialogue( regenerate to drop the original assistant_turn from its prompt context window before that row has been marked superseded (the supersede UPDATE lands at the end so the new event_id is known). + + T90.1: the chat_id filter is pushed into SQL via ``json_extract`` so + ``LIMIT N`` always returns N rows scoped to the requested chat. The + previous implementation filtered chat_id post-fetch in Python, which + let foreign-chat rows fill the LIMIT and yield fewer than N relevant + rows in busy multi-chat databases. """ if exclude_event_id is None: cur = conn.execute( "SELECT id, kind, payload_json FROM event_log " "WHERE kind IN ('user_turn', 'user_turn_edit', 'assistant_turn') " " AND superseded_by IS NULL AND hidden = 0 " + " AND json_extract(payload_json, '$.chat_id') = ? " "ORDER BY id DESC LIMIT ?", - (limit,), + (chat_id, limit), ) else: cur = conn.execute( @@ -69,15 +76,14 @@ def read_recent_dialogue( "WHERE kind IN ('user_turn', 'user_turn_edit', 'assistant_turn') " " AND id != ? " " AND superseded_by IS NULL AND hidden = 0 " + " AND json_extract(payload_json, '$.chat_id') = ? " "ORDER BY id DESC LIMIT ?", - (exclude_event_id, limit), + (exclude_event_id, chat_id, limit), ) rows = list(reversed(cur.fetchall())) out: list[dict] = [] 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( { diff --git a/tests/test_memory_write.py b/tests/test_memory_write.py index 77132ae..8c5253a 100644 --- a/tests/test_memory_write.py +++ b/tests/test_memory_write.py @@ -22,7 +22,7 @@ from chat.db.migrate import apply_migrations from chat.eventlog.log import append_event from chat.eventlog.projector import project from chat.llm.mock import MockLLMClient -from chat.services.memory_write import record_turn_memory, record_turn_memory_for_present +from chat.services.memory_write import record_turn_memory_for_present import chat.state.entities # noqa: F401 - register handlers import chat.state.memory # noqa: F401 import chat.state.world # noqa: F401 @@ -64,14 +64,19 @@ def test_record_turn_memory_writes_event_and_projects(tmp_path): apply_migrations(db) _seed_minimal(db) with open_db(db) as conn: - eid, mid = record_turn_memory( + # T90.3: legacy ``record_turn_memory`` was removed; the unified + # ``record_turn_memory_for_present`` with ``guest_bot_id=None`` + # produces the same single-bot witness mask [1,1,0]. + result = record_turn_memory_for_present( conn, chat_id="chat_bot_a", host_bot_id="bot_a", + guest_bot_id=None, narrative_text="BotA looks up. 'You're back late.'", scene_id=None, chat_clock_at="2026-04-26T20:00:00+00:00", ) + eid, mid = result["bot_a"] assert eid > 0 assert mid is not None and mid > 0 @@ -111,12 +116,15 @@ def test_record_turn_memory_omits_optional_fields(tmp_path): _seed_minimal(db) with open_db(db) as conn: # Call without scene_id/chat_clock_at — should default to None. - eid, mid = record_turn_memory( + # T90.3: migrated from legacy ``record_turn_memory``. + result = record_turn_memory_for_present( conn, chat_id="chat_bot_a", host_bot_id="bot_a", + guest_bot_id=None, narrative_text="A simple memory.", ) + eid, mid = result["bot_a"] assert eid > 0 assert mid is not None and mid > 0 diff --git a/tests/test_regenerate.py b/tests/test_regenerate.py index d8a2d65..b6d5e92 100644 --- a/tests/test_regenerate.py +++ b/tests/test_regenerate.py @@ -757,6 +757,13 @@ def test_regenerate_with_prior_lifecycle_logs_warning(tmp_path, monkeypatch, cap # row's id. assert str(at_id) in msg assert str(completed_id) in msg + # T90.2: wording was tightened from "from superseded turn" to + # "at-or-after turn " — when regenerating an OLDER turn, the + # listed transitions may include legitimate intervening-turn ones + # that stand on their own. The new phrasing avoids implying the + # warning's target turn directly authored every listed transition. + assert "at-or-after turn" in msg + assert "from superseded turn" not in msg def test_regenerate_sibling_lookup_scoped_to_chat(tmp_path, monkeypatch): diff --git a/tests/test_turn_common.py b/tests/test_turn_common.py index 4788fde..3bfc8ff 100644 --- a/tests/test_turn_common.py +++ b/tests/test_turn_common.py @@ -186,6 +186,82 @@ def test_read_recent_dialogue_filters_superseded_and_other_chats(tmp_path): assert ut_id is not None +def test_read_recent_dialogue_limit_respects_chat_scope(tmp_path): + """T90.1: ``read_recent_dialogue`` must push the chat_id filter into + SQL so that ``LIMIT N`` returns N rows scoped to the requested chat — + not N globally-recent rows that may then be filtered down to fewer in + Python. + + Setup: two chats with 60 turns each, interleaved. With the old + post-fetch filter, ``LIMIT 50`` would pull 50 globally-recent rows + (most or all from chat_b — the most recent inserts) and then drop + chat_b ones via the Python check, yielding far fewer than 50 chat_a + rows. After the SQL pushdown, ``LIMIT 50`` should return exactly 50 + chat_a rows. + """ + db = tmp_path / "test.db" + apply_migrations(db) + with open_db(db) as conn: + for chat_id, host_bot in (("chat_a", "bot_a"), ("chat_b", "bot_b")): + append_event( + conn, + kind="bot_authored", + payload={ + "id": host_bot, + "name": host_bot, + "persona": "...", + "voice_samples": [], + "traits": [], + "backstory": "", + "initial_relationship_to_you": "", + "kickoff_prose": "", + }, + ) + append_event( + conn, + kind="chat_created", + payload={ + "id": chat_id, + "host_bot_id": host_bot, + "initial_time": "2026-04-26T20:00:00+00:00", + "narrative_anchor": "Day 1", + "weather": "", + }, + ) + # Interleave 60 user_turn rows in each chat — chat_b's go in last + # so they dominate the global tail. + for i in range(60): + append_event( + conn, + kind="user_turn", + payload={ + "chat_id": "chat_a", + "prose": f"a-{i}", + "segments": [], + }, + ) + for i in range(60): + append_event( + conn, + kind="user_turn", + payload={ + "chat_id": "chat_b", + "prose": f"b-{i}", + "segments": [], + }, + ) + project(conn) + + out = read_recent_dialogue(conn, "chat_a", limit=50) + + # All returned rows should belong to chat_a (texts a-* only). + assert len(out) == 50 + for entry in out: + assert entry["text"].startswith("a-"), ( + f"foreign chat row leaked: {entry!r}" + ) + + def test_gather_prior_edges_fills_missing_with_default(tmp_path): """``gather_prior_edges`` returns one entry per directed pair across ``present_ids``. Missing rows fall back to the schema default