Merge pull request #2725 from langbot-app/fix/session-summary-persistence

fix(monitoring): persist session summaries to avoid history scans

tests/unit_tests/api/service/test_monitoring_executions.py

@@ -403,6 +403,90 @@ async def test_overview_metrics_keep_message_rollup(engine, service):
     assert overview['active_sessions'] == 1
 
 
+@pytest.mark.asyncio
+@pytest.mark.parametrize('filtered', [False, True])
+@pytest.mark.parametrize(
+    ('mode', 'statuses', 'expected'),
+    [
+        ('real', None, (3, 2, 1)),
+        ('debug', None, (0, 0, 0)),
+        ('all', ['completed'], (2, 1, 1)),
+        ('real', ['completed'], (2, 1, 1)),
+        ('real', ['failed'], (0, 0, 0)),
+        ('debug', ['completed'], (0, 0, 0)),
+    ],
+)
+async def test_overview_execution_filters_do_not_read_historical_payload_pages(
+    engine, service, mode, statuses, expected, filtered
+):
+    """Execution membership must read root and link metadata without accessing wide history rows."""
+    import re
+    import sqlalchemy as sa
+
+    await _seed(engine)
+    message = persistence_monitoring.MonitoringMessage
+    payload = 'historical payload ' * 65536
+    async with engine.begin() as connection:
+        await connection.execute(sa.update(message).values(message_content=payload, variables=payload))
+        await connection.execute(
+            sa.update(message).where(message.id == 'msg-assistant').values(parent_message_id='msg-user')
+        )
+        # Vary status selectivity so the planner cannot hide table reads behind a tiny fixture.
+        await connection.execute(
+            sa.insert(message),
+            [
+                dict(
+                    id=f'unlinked-{index}',
+                    workspace_uuid=WORKSPACE,
+                    timestamp=_dt(12),
+                    bot_id='bot-1',
+                    bot_name='Bot',
+                    pipeline_id='pipeline-1',
+                    pipeline_name='Pipeline',
+                    session_id='session-1',
+                    message_content='unlinked assistant output',
+                    role='assistant',
+                    status=('success', 'error', 'discarded', 'skipped', 'rejected', 'running', 'pending')[index % 7],
+                    level='info',
+                )
+                for index in range(64)
+            ],
+        )
+        await connection.exec_driver_sql('ANALYZE')
+
+    statements = []
+
+    def capture(connection, cursor, statement, parameters, context, executemany):
+        if statement.startswith('SELECT'):
+            statements.append((statement, parameters))
+
+    sa.event.listen(engine.sync_engine, 'before_cursor_execute', capture)
+    try:
+        filters = (
+            dict(bot_ids=['bot-1'], pipeline_ids=['pipeline-1'], start_time=_dt(9), end_time=_dt(11))
+            if filtered
+            else {}
+        )
+        overview = await service.get_overview_metrics(WORKSPACE, execution_statuses=statuses, mode=mode, **filters)
+    finally:
+        sa.event.remove(engine.sync_engine, 'before_cursor_execute', capture)
+
+    assert (overview['total_messages'], overview['llm_calls'], overview['active_sessions']) == expected
+    checked_accesses = 0
+    async with engine.connect() as connection:
+        for sql, parameters in statements:
+            # Discover aliases from the executed SQL rather than pinning an alias or index name.
+            sources = {message.__tablename__, *re.findall(r'FROM monitoring_messages AS (\w+)', sql)}
+            plan = (await connection.exec_driver_sql('EXPLAIN QUERY PLAN ' + sql, parameters)).all()
+            accesses = [
+                row[3] for row in plan if row[3].split()[0] in {'SEARCH', 'SCAN'} and row[3].split()[1] in sources
+            ]
+            assert accesses, plan
+            assert all('COVERING INDEX' in access for access in accesses), plan
+            checked_accesses += len(accesses)
+    assert checked_accesses > 0
+
+
 @pytest.mark.asyncio
 async def test_execution_detail_for_agent_and_pipeline(engine, service):
     await _seed(engine)

tests/unit_tests/api/service/test_monitoring_tenancy.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import datetime
+import contextvars
 from types import SimpleNamespace
 
 import pytest
@@ -35,6 +36,12 @@ def _context(workspace_uuid: str) -> ExecutionContext:
 class _PersistenceManager:
     def __init__(self, engine):
         self.engine = engine
+        self.active_transaction = contextvars.ContextVar('monitoring-test-transaction', default=None)
+
+    def tenant_uow(self, workspace):
+        from langbot.pkg.persistence.tenant_uow import TenantUnitOfWork
+
+        return TenantUnitOfWork(self.engine, workspace, active_transaction=self.active_transaction)
 
     async def execute_async(self, *args, **kwargs):
         async with self.engine.connect() as connection:
@@ -112,22 +119,6 @@ async def test_same_session_and_resource_ids_do_not_collide(service):
     context_b = _context(WORKSPACE_B)
     message_a = await _record_message(service, context_a, 'tenant-a')
     message_b = await _record_message(service, context_b, 'tenant-b')
-    await service.record_session_start(
-        context_a,
-        session_id='same-session',
-        bot_id='same-bot',
-        bot_name='Same Bot',
-        pipeline_id='same-pipeline',
-        pipeline_name='Same Pipeline',
-    )
-    await service.record_session_start(
-        context_b,
-        session_id='same-session',
-        bot_id='same-bot',
-        bot_name='Same Bot',
-        pipeline_id='same-pipeline',
-        pipeline_name='Same Pipeline',
-    )
 
     messages_a, total_a = await service.get_messages(context_a)
     messages_b, total_b = await service.get_messages(context_b)
@@ -149,8 +140,9 @@ async def test_session_search_matches_user_id_or_name_within_workspace(service,
         (context_b, 'session-other-workspace', 'customer-42', 'Alice'),
     ]
     for context, session_id, user_id, user_name in fixtures:
-        await service.record_session_start(
+        await service.record_message(
             context,
+            message_content='hello',
             session_id=session_id,
             bot_id='same-bot',
             bot_name='Same Bot',
@@ -221,14 +213,6 @@ async def test_monitoring_queries_and_detail_views_are_strictly_bounded(service)
             'max_offset': 10,
         }
     }
-    await service.record_session_start(
-        context,
-        session_id='same-session',
-        bot_id='same-bot',
-        bot_name='Same Bot',
-        pipeline_id='same-pipeline',
-        pipeline_name='Same Pipeline',
-    )
     message_ids = [await _record_message(service, context, f'message-{index}') for index in range(4)]
     for index in range(3):
         await service.record_llm_call(
@@ -430,14 +414,26 @@ async def test_cloud_cleanup_uses_plan_retention_and_isolates_workspaces(service
     from unittest.mock import AsyncMock
 
     service.ap.deployment = SimpleNamespace(mode='cloud')
-    service.ap.entitlement_resolver = SimpleNamespace(resolve=AsyncMock(return_value=SimpleNamespace(limits={'chat_history.retention_days': 7})))
+    service.ap.entitlement_resolver = SimpleNamespace(
+        resolve=AsyncMock(return_value=SimpleNamespace(limits={'chat_history.retention_days': 7}))
+    )
     old = datetime.datetime.now(datetime.timezone.utc).replace(tzinfo=None) - datetime.timedelta(days=10)
     for workspace in (WORKSPACE_A, WORKSPACE_B):
-        await service.ap.persistence_mgr.execute_async(sqlalchemy.insert(MonitoringMessage).values(
-            id=f'policy-{workspace}', workspace_uuid=workspace, timestamp=old,
-            bot_id='bot', bot_name='Bot', pipeline_id='pipeline', pipeline_name='Pipeline',
-            message_content='history', session_id='session', status='success', level='info',
-        ))
+        await service.ap.persistence_mgr.execute_async(
+            sqlalchemy.insert(MonitoringMessage).values(
+                id=f'policy-{workspace}',
+                workspace_uuid=workspace,
+                timestamp=old,
+                bot_id='bot',
+                bot_name='Bot',
+                pipeline_id='pipeline',
+                pipeline_name='Pipeline',
+                message_content='history',
+                session_id='session',
+                status='success',
+                level='info',
+            )
+        )
     deleted = await service.cleanup_expired_records(_context(WORKSPACE_A), retention_days=90)
     assert deleted['monitoring_messages'] == 1
     rows = await service.ap.persistence_mgr.execute_async(sqlalchemy.select(MonitoringMessage.workspace_uuid))

tests/unit_tests/api/test_mcp_server.py

@@ -4,7 +4,9 @@ from types import SimpleNamespace
 from unittest.mock import AsyncMock
 
 import pytest
+from mcp.server.fastmcp.exceptions import ToolError
 
+from langbot.pkg.api.mcp import server as mcp_server
 from langbot.pkg.api.mcp.server import LangBotMCPServer
 
 
@@ -77,3 +79,29 @@ async def test_mcp_server_exposes_bot_event_route_tools():
         'th_TH',
         'vi_VN',
     }
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+    'filters', [{}, {'bot_ids': ['bot-1'], 'user_query': 'Alice'}, {'mode': 'real'}, {'execution_statuses': ['failed']}]
+)
+async def test_monitoring_sessions_accept_only_conversation_filters(monkeypatch, filters):
+    app = _make_app()
+    app.monitoring_service = SimpleNamespace(
+        normalize_page_window=lambda limit, offset: (limit, offset),
+        get_sessions=AsyncMock(return_value=([], 0)),
+    )
+    context = SimpleNamespace(workspace_uuid='workspace-1')
+    monkeypatch.setattr(mcp_server, 'get_request_context', lambda: context)
+    monkeypatch.setattr(mcp_server, '_authorized', lambda permission: context)
+    monkeypatch.setattr(mcp_server, 'get_workspace_language', AsyncMock(return_value='en_US'))
+    server = LangBotMCPServer(app)
+
+    arguments = {'kind': 'sessions', 'filters': filters}
+    if set(filters) & {'mode', 'execution_statuses'}:
+        with pytest.raises(ToolError, match='Unsupported monitoring filter'):
+            await server.mcp.call_tool('get_monitoring_records', arguments)
+        app.monitoring_service.get_sessions.assert_not_awaited()
+    else:
+        await server.mcp.call_tool('get_monitoring_records', arguments)
+        app.monitoring_service.get_sessions.assert_awaited_once_with(context, limit=100, offset=0, **filters)