From 202df9dcde40fb338d53146217d75a2f94865721 Mon Sep 17 00:00:00 2001 From: Ethan Date: Wed, 3 Jun 2026 15:22:51 +1000 Subject: [PATCH] Fix HTTP 500 in history routes: order ChatMessage by timestamp, not created_at (#1673) The mark-stopped, update-last-meta, and merge-last-assistant handlers in routes/history_routes.py ordered ChatMessage queries by DbChatMessage.created_at. ChatMessage does not inherit TimestampMixin and has only a `timestamp` column, so SQLAlchemy raised AttributeError at query-build time -> HTTP 500 on Stop, last-message metadata updates, and Continue/merge. Each handler mutates in-memory history before the failing query, so a failed request also silently diverged the in-memory view from the database. Order by DbChatMessage.timestamp (already used elsewhere in the file and covered by the ix_messages_session_time index). Add a regression test pinning the model column reality, the corrected query, and a guard against re-introducing created_at. Fixes #1659 Co-authored-by: Ethan <23321960+0xLeathery@users.noreply.github.com> --- ...t_history_order_by_timestamp_regression.py | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 tests/test_history_order_by_timestamp_regression.py diff --git a/tests/test_history_order_by_timestamp_regression.py b/tests/test_history_order_by_timestamp_regression.py new file mode 100644 index 0000000..3fb2922 --- /dev/null +++ b/tests/test_history_order_by_timestamp_regression.py @@ -0,0 +1,77 @@ +"""Regression guard for #1659. + +`routes/history_routes.py` ordered three ChatMessage queries by +``DbChatMessage.created_at`` — the mark-stopped (`:268`), update-last-meta +(`:323`) and merge-last-assistant (`:404`) handlers. The ``ChatMessage`` model +does **not** inherit ``TimestampMixin`` and exposes only a ``timestamp`` column, +so ``DbChatMessage.created_at`` raised ``AttributeError`` at query-build time -> +HTTP 500 on Stop, last-message metadata updates, and Continue/merge. + +This test pins three things: + 1. the model genuinely has ``timestamp`` and no ``created_at`` (justifies the fix); + 2. the corrected ``order_by(DbChatMessage.timestamp)`` query builds and runs; + 3. ``routes/history_routes.py`` never orders a ChatMessage query by the + non-existent ``created_at`` column again. +""" +import os +from pathlib import Path + +# Keep the import-time engine hermetic — no on-disk app.db. +os.environ.setdefault("DATABASE_URL", "sqlite:///:memory:") + +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker + +from core.database import Base, ChatMessage as DbChatMessage, Session as DbSession + + +HISTORY_ROUTES = Path(__file__).resolve().parent.parent / "routes" / "history_routes.py" + + +def test_chatmessage_model_has_timestamp_not_created_at(): + assert hasattr(DbChatMessage, "timestamp"), "ChatMessage should expose a `timestamp` column" + assert not hasattr(DbChatMessage, "created_at"), ( + "ChatMessage does not inherit TimestampMixin; ordering by `created_at` " + "raises AttributeError -> HTTP 500 (#1659)" + ) + + +def test_order_by_timestamp_query_executes(): + engine = create_engine("sqlite:///:memory:") + Base.metadata.create_all(engine) + db = sessionmaker(bind=engine)() + try: + sid = "sess1234" + # FK enforcement is on (PRAGMA foreign_keys), so seed the parent session. + db.add(DbSession(id=sid, name="t", endpoint_url="http://x", model="m")) + db.add(DbChatMessage(id="m1", session_id=sid, role="assistant", content="first")) + db.add(DbChatMessage(id="m2", session_id=sid, role="assistant", content="second")) + db.commit() + + # Mirrors mark_stopped / update_last_meta (descending, .first()). + last_assistant = ( + db.query(DbChatMessage) + .filter(DbChatMessage.session_id == sid, DbChatMessage.role == "assistant") + .order_by(DbChatMessage.timestamp.desc()) + .first() + ) + assert last_assistant is not None + + # Mirrors merge_last_assistant (ascending, .all()). + all_rows = ( + db.query(DbChatMessage) + .filter(DbChatMessage.session_id == sid) + .order_by(DbChatMessage.timestamp) + .all() + ) + assert len(all_rows) == 2 + finally: + db.close() + + +def test_history_routes_do_not_order_by_created_at(): + text = HISTORY_ROUTES.read_text(encoding="utf-8") + assert "DbChatMessage.created_at" not in text, ( + "history_routes must order ChatMessage queries by `.timestamp`, not the " + "non-existent `.created_at` column (raises AttributeError -> HTTP 500, #1659)" + )