Files
odysseus/tests/test_fork_session_metadata.py
T
RaresKeY d449a9d431 fix(history): defer full transcript hydration to model sends (#5929)
* fix(history): defer full hydration to model sends

* fix(session): key hydration on real rows, fork through get_session

Two regressions from the display/model-context split, both reproducible
against dev.

The hydration gate compared the cached transcript against the
denormalized sessions.message_count column. That column drifts in normal
operation — _persist_message swallows a failed insert while add_message
has already appended in memory, so the next successful persist writes
rows+1 — and _db_to_session re-read the same column after each reload, so
the shortfall never closed. Every send, edit, delete and truncate on a
warm session re-selected the whole message table: the cost this change
set out to remove, relocated onto the hot path. The other direction was
just as bad — a persist for an uncached session writes message_count = 0,
and a stale-low counter with a partly filled cache meant no hydration at
all and a silently truncated transcript for the model.

sync_session_metadata now reconciles message_count against COUNT(*) on
chat_messages (one indexed count inside the connection it already opens),
and _db_to_session trusts the rows it just loaded. A hydrate always
closes the gap, so the next read is a cache hit.

fork_session read session_manager.sessions directly and never hydrated.
keep_count indexes into source.history, and display pagination no longer
fills that cache, so forking after a restart returned HTTP 200 with an
empty conversation and no error surfaced. It goes through get_session
now.

_hydrate_session_history_from_db is gone with its helper: get_session is
the hydration seam, and rebuilding session.history from raw rows in the
display fallback overwrote the parsed multimodal content and the _db_id
edit/delete keys that had just been set.

Tests drive a real SessionManager over a temp DB instead of a stub that
only proved the stub hydrates — both drift directions, the send path
warm and cold, and a fork taken after a restart. All five fail without
this change. The brittle SQL-text assertions are dropped; the page
bounds are already proven by the response body.

* fix(history): route pagination through canonical handler

---------

Co-authored-by: Léo <leograndcontact@gmail.com>
2026-08-10 19:39:21 +01:00

90 lines
3.0 KiB
Python

"""Forking a session must not mutate the source session's messages.
ChatMessage.metadata is a dict. add_message() -> _persist_message() stamps
_db_id (and timestamp) onto that dict in place. The fork handler used to pass
the source message's metadata dict by reference into the new session, so
persisting the fork rewrote the SOURCE messages' _db_id — breaking
edit/delete-by-id on the original conversation. The fork must copy the dict.
"""
import asyncio
from types import SimpleNamespace
from core.models import ChatMessage
import routes.history_routes as mod
class _FakeSession:
def __init__(self, name="", owner=None):
self.name = name
self.owner = owner
self.endpoint_url = ""
self.model = ""
self.history = []
def add_message(self, message):
# Mirror _persist_message: stamp the in-memory message's metadata.
if message.metadata is None:
message.metadata = {}
message.metadata["_db_id"] = f"new-{len(self.history)}"
self.history.append(message)
class _FakeSessionManager:
def __init__(self, source):
self.sessions = {"src-id": source}
self.created = None
def get_session(self, session_id):
# Fork looks the source up through get_session — the hydration seam —
# so a session only present in the DB still forks a real transcript.
return self.sessions[session_id]
def create_session(self, session_id=None, name=None, endpoint_url=None,
model=None, rag=False, owner=None):
self.created = _FakeSession(name=name, owner=owner)
return self.created
def save_sessions(self):
pass
def _fork_handler(router):
for route in router.routes:
if "/fork" in getattr(route, "path", "") and "POST" in getattr(route, "methods", set()):
return route.endpoint
raise AssertionError("fork route not found")
def test_fork_does_not_corrupt_source_message_metadata(monkeypatch):
monkeypatch.setattr(mod, "_verify_session_owner", lambda *a, **k: None)
source = _FakeSession(name="Original", owner="alice")
source.history = [
ChatMessage("user", "hi", {"_db_id": "src-0"}),
ChatMessage("assistant", "yo", {"_db_id": "src-1"}),
]
sm = _FakeSessionManager(source)
req = SimpleNamespace()
async def _json():
return {"keep_count": 2}
req.json = _json
router = mod.setup_history_routes(sm)
fork = _fork_handler(router)
result = asyncio.run(fork(request=req, session_id="src-id"))
assert result["status"] == "ok"
assert result["kept"] == 2
# The forked session got its own metadata dicts...
new_session = sm.created
assert new_session.history[0].metadata is not source.history[0].metadata
assert new_session.history[1].metadata is not source.history[1].metadata
# ...and the source session's _db_id values are untouched.
assert source.history[0].metadata["_db_id"] == "src-0"
assert source.history[1].metadata["_db_id"] == "src-1"