From ad3e309074e0d75fa73d57aa9a178b8f98603b67 Mon Sep 17 00:00:00 2001 From: robyroro <41092915+robyroro@users.noreply.github.com> Date: Thu, 8 Oct 2026 06:48:41 +0300 Subject: [PATCH 1/2] fix(chats): preserve saved conversations during tidy --- routes/session_routes.py | 47 +----- src/session_actions.py | 59 +------- tests/test_chat_tidy_content_preservation.py | 148 +++++++++++++++++++ tests/test_session_actions_cleanup.py | 7 +- tests/test_session_list_owner_scope.py | 22 +-- 5 files changed, 180 insertions(+), 103 deletions(-) create mode 100644 tests/test_chat_tidy_content_preservation.py diff --git a/routes/session_routes.py b/routes/session_routes.py index fa687ee14..c47be42e2 100644 --- a/routes/session_routes.py +++ b/routes/session_routes.py @@ -1305,7 +1305,7 @@ def setup_session_routes( def auto_sort_sessions(request: Request, skip_llm: bool = False): """Use AI to categorize all sessions into folders. - Phase 1 deletes empty/throwaway sessions and Phase 2 asks the LLM + Phase 1 deletes empty/Incognito sessions and Phase 2 asks the LLM to assign folders. When `skip_llm=true` the endpoint returns after Phase 1 — used by the "Tidy (no AI)" UI affordance so users can clean junk without spending tokens. @@ -1315,21 +1315,11 @@ def setup_session_routes( single_user_mode = not user and _auth_disabled() user_sessions = session_manager.get_sessions_for_user(user) - # Delete empty and throwaway sessions before sorting + # Saved conversations are never disposable based on their title or prompt. from core.database import ChatMessage as DbMsg db = SessionLocal() deleted_empty = 0 deleted_throwaway = 0 - # Names that indicate a throwaway/test session (case-insensitive exact or prefix match) - _THROWAWAY_NAMES = { - "test", "testing", "asdf", "asd", "hello", "hi", "hey", - "yo", "sup", "hola", "hii", "hiii", "heyo", - "foo", "bar", "baz", "tmp", "temp", "scratch", "untitled", - "new chat", "delete", "remove", "junk", "trash", "xxx", - "abc", "qwerty", "blah", "stuff", "whatever", "idk", - "ok", "lol", "bruh", "hmm", "hm", "meh", - } - _THROWAWAY_MAX_MESSAGES = 4 # only delete if <= this many messages try: rows_q = db.query(DbSession).filter(DbSession.archived == False) if user: @@ -1338,15 +1328,11 @@ def setup_session_routes( rows_q = rows_q.filter(DbSession.owner == user) rows = rows_q.limit(2000).all() folder_map = {r.id: r.folder for r in rows} - # Precompute per-session message counts in TWO aggregate queries + # Precompute per-session message counts in one aggregate query # instead of 1–3 queries PER session — with many chats the per-row # loop was doing thousands of round-trips and blowing the timeout. from sqlalchemy import func as _sa_func _counts = dict(db.query(DbMsg.session_id, _sa_func.count(DbMsg.id)).group_by(DbMsg.session_id).all()) - _asst_counts = dict( - db.query(DbMsg.session_id, _sa_func.count(DbMsg.id)) - .filter(DbMsg.role == "assistant").group_by(DbMsg.session_id).all() - ) cleanup_now = utcnow_naive() for row in rows: # Never delete important sessions @@ -1356,9 +1342,10 @@ def setup_session_routes( if (row.name or "").strip() == "Incognito": should_delete = True deleted_throwaway += 1 - db.delete(row) if hasattr(session_manager, 'delete_session'): session_manager.delete_session(row.id) + else: + db.delete(row) continue if is_session_recently_active(row, now=cleanup_now): continue @@ -1367,31 +1354,11 @@ def setup_session_routes( if msg_count == 0: should_delete = True deleted_empty += 1 - elif msg_count <= _THROWAWAY_MAX_MESSAGES: - name = (row.name or "").strip().lower() - # Check first user message content (AI renames sessions, so - # "hi" becomes "Casual Greeting Exchange" — name alone won't match) - first_msg = db.query(DbMsg.content).filter( - DbMsg.session_id == row.id, DbMsg.role == "user" - ).order_by(DbMsg.timestamp).first() - first_text = (first_msg[0] or "").strip().lower() if first_msg else "" - # Count assistant messages — if user sent something but AI never replied, it's dead - assistant_count = _asst_counts.get(row.id, 0) - if name in _THROWAWAY_NAMES or name.startswith("chat:") or first_text in _THROWAWAY_NAMES: - should_delete = True - deleted_throwaway += 1 - # Single user message with no AI response = dead session - elif msg_count == 1 and assistant_count == 0: - should_delete = True - deleted_throwaway += 1 - # Short phrase (1-3 words) with no real AI conversation (<=2 msgs) - elif msg_count <= 2 and first_text and len(first_text.split()) <= 3 and len(first_text) <= 40: - should_delete = True - deleted_throwaway += 1 if should_delete: - db.delete(row) if hasattr(session_manager, 'delete_session'): session_manager.delete_session(row.id) + else: + db.delete(row) if deleted_empty or deleted_throwaway: db.commit() logger.info(f"Auto-sort: deleted {deleted_empty} empty + {deleted_throwaway} throwaway sessions") diff --git a/src/session_actions.py b/src/session_actions.py index 072bb4c06..d59979cec 100644 --- a/src/session_actions.py +++ b/src/session_actions.py @@ -12,16 +12,6 @@ from datetime import datetime, timedelta, timezone logger = logging.getLogger(__name__) -# Names that indicate a throwaway/test session -_THROWAWAY_NAMES = { - "test", "testing", "asdf", "asd", "hello", "hi", "hey", - "yo", "sup", "hola", "hii", "hiii", "heyo", - "foo", "bar", "baz", "tmp", "temp", "scratch", "untitled", - "new chat", "delete", "remove", "junk", "trash", "xxx", - "abc", "qwerty", "blah", "stuff", "whatever", "idk", - "ok", "lol", "bruh", "hmm", "hm", "meh", -} -_THROWAWAY_MAX_MESSAGES = 4 _FRESH_EMPTY_SESSION_GRACE = timedelta(minutes=10) _FRESH_SESSION_GRACE = _FRESH_EMPTY_SESSION_GRACE @@ -54,14 +44,15 @@ def is_session_recently_active(row, now=None, grace=_FRESH_SESSION_GRACE) -> boo async def run_auto_sort(owner: str, skip_llm: bool = False, delete_throwaway: bool = True) -> str: - """Run session cleanup + (optional) AI folder sort for the given owner. + """Remove empty/Incognito sessions and optionally organize saved chats. Args: owner: user whose sessions to process - skip_llm: when True, do only Phase 1 (delete empty/throwaway sessions); + skip_llm: when True, do only Phase 1 (delete empty/Incognito sessions); skip Phase 2 (AI folder assignment). Used by the built-in daily background sweep so it never burns LLM tokens. - delete_throwaway: when False, only empty/incognito sessions are deleted. + delete_throwaway: retained for caller compatibility. Ordinary sessions + with saved messages are always preserved, regardless of this flag. Returns a human-readable summary of what was done. """ @@ -71,7 +62,7 @@ async def run_auto_sort(owner: str, skip_llm: bool = False, delete_throwaway: bo db = SessionLocal() try: - # ── Phase 1: Delete empty/throwaway sessions ── + # ── Phase 1: Delete only empty and explicitly Incognito sessions ── deleted_empty = 0 deleted_throwaway = 0 @@ -93,47 +84,13 @@ async def run_auto_sort(owner: str, skip_llm: bool = False, delete_throwaway: bo if is_session_recently_active(row, now=cleanup_now): continue - msg_count = db.query(DbMsg.id).filter( + has_messages = db.query(DbMsg.id).filter( DbMsg.session_id == row.id - ).limit(_THROWAWAY_MAX_MESSAGES + 1).count() - should_delete = False - - if msg_count == 0: + ).first() is not None + if not has_messages: if is_fresh: continue - should_delete = True deleted_empty += 1 - elif delete_throwaway and msg_count <= _THROWAWAY_MAX_MESSAGES: - name = (row.name or "").strip().lower() - first_msg = db.query(DbMsg.content).filter( - DbMsg.session_id == row.id, DbMsg.role == "user" - ).order_by(DbMsg.timestamp).first() - first_text = (first_msg[0] or "").strip().lower() if first_msg else "" - assistant_count = db.query(DbMsg.id).filter( - DbMsg.session_id == row.id, DbMsg.role == "assistant" - ).limit(1).count() - - if name in _THROWAWAY_NAMES or name.startswith("chat:") or first_text in _THROWAWAY_NAMES: - should_delete = True - deleted_throwaway += 1 - elif msg_count == 1 and assistant_count == 0: - should_delete = True - deleted_throwaway += 1 - elif msg_count <= 4 and first_text and len(first_text.split()) <= 8 and len(first_text) <= 80: - # Short trivial chats — e.g. "write hi to a friend" → "Hi!" - should_delete = True - deleted_throwaway += 1 - else: - # Aggressive: total message text under 250 chars combined = trivial - msg_rows = db.query(DbMsg.content).filter( - DbMsg.session_id == row.id - ).all() - total_chars = sum(len(m[0] or "") for m in msg_rows) - if total_chars <= 250: - should_delete = True - deleted_throwaway += 1 - - if should_delete: db.delete(row) if deleted_empty or deleted_throwaway: diff --git a/tests/test_chat_tidy_content_preservation.py b/tests/test_chat_tidy_content_preservation.py new file mode 100644 index 000000000..6c2260a98 --- /dev/null +++ b/tests/test_chat_tidy_content_preservation.py @@ -0,0 +1,148 @@ +"""Both chat tidy entry points retain saved conversations and still remove empties.""" +import asyncio +from datetime import datetime, timedelta, timezone + +import pytest +from fastapi import APIRouter, FastAPI +from fastapi.testclient import TestClient +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker + +import core.database as database +import core.session_manager as manager_module +import routes.session_routes as routes +import src.session_actions as actions + + +@pytest.fixture +def tidy_api(tmp_path, monkeypatch): + engine = create_engine( + f"sqlite:///{(tmp_path / 'tidy.db').as_posix()}", + connect_args={"check_same_thread": False}, + ) + database.Base.metadata.create_all(engine) + factory = sessionmaker(bind=engine, autoflush=False) + monkeypatch.setattr(database, "SessionLocal", factory) + monkeypatch.setattr(manager_module, "SessionLocal", factory) + monkeypatch.setattr(routes, "SessionLocal", factory) + monkeypatch.setattr(routes, "router", APIRouter(prefix="/api")) + monkeypatch.setattr(routes, "effective_user", lambda request: "alice") + manager = manager_module.SessionManager.__new__(manager_module.SessionManager) + manager.sessions = {} + manager.upload_handler = None + monkeypatch.setattr(manager, "get_sessions_for_user", lambda owner: {}) + app = FastAPI() + app.include_router(routes.setup_session_routes(manager, {})) + with TestClient(app) as client: + yield client, factory, manager + engine.dispose() + + +def run_tidy(client, entrypoint): + if entrypoint == "action": + return asyncio.run(actions.run_auto_sort("alice", skip_llm=True)) + response = client.post("/api/sessions/auto-sort?skip_llm=true") + assert response.status_code == 200, response.text + return response.json() + + +@pytest.mark.parametrize("entrypoint", ["route", "action"]) +@pytest.mark.parametrize("title,prompt", [ + ("New chat", "Please analyze the following contract and retain all details. " * 20), + ("Contract analysis", "Review contract"), + ("Chat: analysis", "Explain these results and retain the full answer"), + ("Test", "Here is a large input that should be analyzed and retained"), +], ids=["generic-title", "short-prompt", "title-prefix", "test-title"]) +def test_tidy_preserves_nontrivial_saved_answer(tidy_api, title, prompt, entrypoint): + client, factory, manager = tidy_api + old = datetime.now(timezone.utc).replace(tzinfo=None) - timedelta(hours=2) + answer = "Clause analysis and recommended changes.\n" * 2000 + with factory() as db: + db.add(database.Session( + id="valuable-chat", name=title, owner="alice", endpoint_url="", model="", + archived=False, is_important=False, message_count=2, + created_at=old, updated_at=old, last_accessed=old, last_message_at=old, + )) + db.commit() + db.add_all([ + database.ChatMessage(id="prompt", session_id="valuable-chat", role="user", + content=prompt, timestamp=old), + database.ChatMessage(id="answer", session_id="valuable-chat", role="assistant", + content=answer, timestamp=old + timedelta(seconds=1)), + ]) + db.commit() + run_tidy(client, entrypoint) + with factory() as db: + remaining = db.query(database.ChatMessage).filter_by(session_id="valuable-chat").count() + survives = db.get(database.Session, "valuable-chat") is not None + assert survives and remaining == 2, "Title/prompt heuristics must not erase saved answers" + with factory() as db: + assert db.get(database.ChatMessage, "answer").content == answer + + +@pytest.mark.parametrize("entrypoint", ["route", "action"]) +@pytest.mark.parametrize("kind", ["unanswered", "greeting", "empty", "fresh-empty", "starred-empty", "archived", "other-owner", "incognito"]) +def test_tidy_cleanup_boundaries(tidy_api, entrypoint, kind): + client, factory, manager = tidy_api + now = datetime.now(timezone.utc).replace(tzinfo=None) + timestamp = now if kind == "fresh-empty" else now - timedelta(hours=2) + message = kind in {"unanswered", "greeting", "archived", "other-owner", "incognito"} + with factory() as db: + db.add(database.Session( + id="boundary-chat", name="Incognito" if kind == "incognito" else "New chat", + owner="bob" if kind == "other-owner" else "alice", endpoint_url="", model="", + archived=kind == "archived", is_important=kind == "starred-empty", + # Deliberately inaccurate metadata: cleanup must use message rows. + message_count=0 if message else 40, + created_at=timestamp, updated_at=timestamp, last_accessed=timestamp, last_message_at=timestamp, + )) + db.commit() + if message: + db.add(database.ChatMessage(id="boundary-message", session_id="boundary-chat", role="user", + content="hi" if kind == "greeting" else "Pending question " * 100, + timestamp=timestamp)) + db.commit() + run_tidy(client, entrypoint) + deleted = kind in {"empty", "incognito"} + with factory() as db: + assert (db.get(database.Session, "boundary-chat") is None) == deleted + assert db.query(database.ChatMessage).filter_by(session_id="boundary-chat").count() == int(message and not deleted) + + +@pytest.mark.parametrize("entrypoint", ["route", "action"]) +def test_tidy_keeps_folder_assignment_working(tidy_api, monkeypatch, entrypoint): + from types import SimpleNamespace + import src.llm_core as llm + import src.task_endpoint as endpoints + client, factory, manager = tidy_api + old = datetime.now(timezone.utc).replace(tzinfo=None) - timedelta(hours=2) + for sid in ["chat-one-long-id", "chat-two-long-id"]: + with factory() as db: + db.add(database.Session(id=sid, name="New chat", owner="alice", endpoint_url="", model="", + archived=False, created_at=old, updated_at=old, + last_accessed=old, last_message_at=old)) + db.commit() + db.add(database.ChatMessage(id=f"{sid}-message", session_id=sid, role="user", + content="Saved question", timestamp=old)) + db.commit() + manager.sessions[sid] = SimpleNamespace(id=sid, name="New chat", archived=False, + updated_at=old.isoformat(), created_at=old.isoformat()) + monkeypatch.setattr(manager, "get_sessions_for_user", lambda owner: manager.sessions) + monkeypatch.setattr(endpoints, "resolve_task_endpoint", lambda **kwargs: ("http://model.invalid", "test", {})) + response_text = '{"folders":{"Projects":["chat-one","chat-two"]}}' + monkeypatch.setattr(llm, "llm_call", lambda *args, **kwargs: response_text) + + async def async_response(*args, **kwargs): + return response_text + + monkeypatch.setattr(llm, "llm_call_async", async_response) + if entrypoint == "action": + result = asyncio.run(actions.run_auto_sort("alice")) + assert "Sorted 2 sessions" in result + else: + response = client.post("/api/sessions/auto-sort") + assert response.status_code == 200, response.text + assert response.json()["updated"] == 2 + with factory() as db: + assert db.query(database.ChatMessage).count() == 2 + assert {row.folder for row in db.query(database.Session).all()} == {"Projects"} diff --git a/tests/test_session_actions_cleanup.py b/tests/test_session_actions_cleanup.py index 221713d33..be89a29de 100644 --- a/tests/test_session_actions_cleanup.py +++ b/tests/test_session_actions_cleanup.py @@ -128,7 +128,7 @@ def test_auto_sort_keeps_fresh_session_while_first_response_is_pending(monkeypat db.close() -def test_auto_sort_still_deletes_old_throwaway_sessions(monkeypatch): +def test_auto_sort_keeps_old_nonempty_sessions_even_with_throwaway_title(monkeypatch): session_factory = _make_session_factory() _install_session_factory(monkeypatch, session_factory) @@ -160,7 +160,8 @@ def test_auto_sort_still_deletes_old_throwaway_sessions(monkeypatch): db = session_factory() try: - assert db.query(DbSession).filter(DbSession.id == sid).first() is None - assert "Cleaned 1 sessions" in result + assert db.query(DbSession).filter(DbSession.id == sid).first() is not None + assert db.query(DbMessage).filter(DbMessage.session_id == sid).count() == 1 + assert "Cleaned 0 sessions" in result finally: db.close() diff --git a/tests/test_session_list_owner_scope.py b/tests/test_session_list_owner_scope.py index fc973bd53..aacde8d29 100644 --- a/tests/test_session_list_owner_scope.py +++ b/tests/test_session_list_owner_scope.py @@ -19,6 +19,7 @@ from core.database import ChatMessage as DbMessage from core.database import Session as DbSession _TMPDB = tempfile.NamedTemporaryFile(suffix=".db", delete=False) +_TMPDB.close() # SQLite owns its connections; release the Windows file handle. _ENGINE = create_engine( f"sqlite:///{_TMPDB.name}", connect_args={"check_same_thread": False}, @@ -230,12 +231,14 @@ def test_patch_session_updates_persisted_cwd(monkeypatch): def test_auto_sort_skip_llm_cleans_owner_stamped_sessions_when_auth_disabled(monkeypatch): import routes.session_routes as sr + from fastapi import APIRouter from unittest.mock import MagicMock _stub_multipart_if_missing(monkeypatch) monkeypatch.setenv("AUTH_ENABLED", "false") monkeypatch.setattr(sr, "SessionLocal", _TS) monkeypatch.setattr(sr, "effective_user", lambda request: None) + monkeypatch.setattr(sr, "router", APIRouter(prefix="/api")) sid = str(uuid.uuid4()) old_time = cdb.utcnow_naive() - timedelta(hours=2) @@ -250,19 +253,12 @@ def test_auto_sort_skip_llm_cleans_owner_stamped_sessions_when_auth_disabled(mon endpoint_url="http://localhost", model="gpt-4", archived=False, - message_count=1, + message_count=0, created_at=old_time, updated_at=old_time, last_message_at=old_time, last_accessed=old_time, )) - db.add(DbMessage( - id="m-" + uuid.uuid4().hex, - session_id=sid, - role="user", - content="hi", - timestamp=old_time, - )) db.commit() finally: db.close() @@ -270,6 +266,13 @@ def test_auto_sort_skip_llm_cleans_owner_stamped_sessions_when_auth_disabled(mon session = MagicMock(id=sid, name="New chat", model="gpt-4", endpoint_url="http://localhost", rag=False, archived=False) sm = MagicMock() sm.get_sessions_for_user.return_value = {sid: session} + + def delete_session(session_id): + with _TS() as db: + db.query(DbSession).filter(DbSession.id == session_id).delete() + db.commit() + + sm.delete_session.side_effect = delete_session router = sr.setup_session_routes(sm, {}) endpoint = next(r.endpoint for r in router.routes if getattr(r, "path", "") == "/api/sessions/auto-sort" @@ -277,7 +280,8 @@ def test_auto_sort_skip_llm_cleans_owner_stamped_sessions_when_auth_disabled(mon result = endpoint(request=MagicMock(), skip_llm=True) - assert result["deleted_throwaway"] == 1 + assert result["deleted_empty"] == 1 + assert result["deleted_throwaway"] == 0 db = _TS() try: assert db.query(DbSession).filter(DbSession.id == sid).first() is None From b1c3125865a452a20ffc9896a31f34f967d424fe Mon Sep 17 00:00:00 2001 From: Alexandre Teixeira Date: Fri, 30 Oct 2026 15:00:08 +0000 Subject: [PATCH 2/2] test: accept positional arguments in task endpoint mock for auto-sort --- tests/test_chat_tidy_content_preservation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_chat_tidy_content_preservation.py b/tests/test_chat_tidy_content_preservation.py index 6c2260a98..b203eac5f 100644 --- a/tests/test_chat_tidy_content_preservation.py +++ b/tests/test_chat_tidy_content_preservation.py @@ -128,7 +128,7 @@ def test_tidy_keeps_folder_assignment_working(tidy_api, monkeypatch, entrypoint) manager.sessions[sid] = SimpleNamespace(id=sid, name="New chat", archived=False, updated_at=old.isoformat(), created_at=old.isoformat()) monkeypatch.setattr(manager, "get_sessions_for_user", lambda owner: manager.sessions) - monkeypatch.setattr(endpoints, "resolve_task_endpoint", lambda **kwargs: ("http://model.invalid", "test", {})) + monkeypatch.setattr(endpoints, "resolve_task_endpoint", lambda *args, **kwargs: ("http://model.invalid", "test", {})) response_text = '{"folders":{"Projects":["chat-one","chat-two"]}}' monkeypatch.setattr(llm, "llm_call", lambda *args, **kwargs: response_text)