Merge pull request #2672 from langbot-app/test/quality-runtime-oracles-20261017

test(runtime): assert filter and message delivery semantics

tests/integration/api/test_bots.py

@@ -201,8 +201,14 @@ class TestBotEndpoints:
         assert response.status_code == 200
         data = await response.get_json()
         assert data['code'] == 0
-        assert 'data' in data
-        assert 'bots' in data['data']
+        assert data['data']['bots'] == [
+            {
+                'uuid': 'test-bot-uuid',
+                'name': 'Test Bot',
+                'platform': 'telegram',
+                'pipeline_uuid': 'test-pipeline-uuid',
+            }
+        ]
 
     @pytest.mark.asyncio
     async def test_create_bot_success(self, quart_test_client):

tests/unit_tests/pipeline/test_aggregator.py

@@ -560,19 +560,18 @@ class TestMessageAggregatorAddMessage:
 
         agg = aggregator.MessageAggregator(app)
 
-        chain = text_chain('hello')
-        event = friend_message_event(chain)
         adapter = mock_adapter()
 
         # Add messages up to MAX_BUFFER_MESSAGES
         for i in range(aggregator.MAX_BUFFER_MESSAGES):
+            chain = text_chain(f'message {i}')
             await agg.add_message(
                 execution_context=execution_context(pipeline_uuid='test-pipeline'),
                 bot_uuid='test-bot',
                 launcher_type=provider_session.LauncherTypes.PERSON,
                 launcher_id=12345,
                 sender_id=12345,
-                message_event=event,
+                message_event=friend_message_event(chain),
                 message_chain=chain,
                 adapter=adapter,
                 pipeline_uuid='test-pipeline',
@@ -588,6 +587,14 @@ class TestMessageAggregatorAddMessage:
             'test-pipeline',
         )
         assert session_id not in agg.buffers or len(agg.buffers[session_id].messages) == 0
+        app.query_pool.add_query.assert_awaited_once()
+        forwarded = app.query_pool.add_query.call_args.kwargs
+        assert str(forwarded['message_chain']) == '\n'.join(
+            f'message {i}' for i in range(aggregator.MAX_BUFFER_MESSAGES)
+        )
+        assert forwarded['execution_context'] == context
+        assert forwarded['bot_uuid'] == 'test-bot'
+        assert forwarded['pipeline_uuid'] == 'test-pipeline'
 
 
 class TestMessageAggregatorMerge:

tests/unit_tests/pipeline/test_chat_handler.py

@@ -189,7 +189,7 @@ class TestChatMessageHandlerReal:
     async def test_user_message_alter_string(self, fake_app, mock_event_ctx, set_runner):
         """user_message_alter as string updates query.user_message."""
         from tests.factories import text_query
-        from langbot_plugin.api.entities.builtin.provider.message import Message
+        from langbot_plugin.api.entities.builtin.provider.message import Message, ContentElement
 
         chat = get_chat_handler()
 
@@ -200,7 +200,8 @@ class TestChatMessageHandlerReal:
         query = text_query('original')
         query.adapter = Mock()
         query.adapter.is_stream_output_supported = AsyncMock(return_value=False)
-        query.user_message = Message(role='user', content=[])
+        query.user_message = Message(role='user', content=[ContentElement.from_text('original')])
+        received_contents = []
 
         class QuickRunner:
             name = 'local-agent'
@@ -210,6 +211,7 @@ class TestChatMessageHandlerReal:
                 self.config = config
 
             async def run(self, query):
+                received_contents.append(query.user_message.content)
                 yield Message(role='assistant', content='ok')
 
         set_runner(QuickRunner)
@@ -220,7 +222,9 @@ class TestChatMessageHandlerReal:
         async for result in handler.handle(query):
             results.append(result)
 
-        assert query.user_message.content is not None
+        expected_content = [ContentElement.from_text('altered text')]
+        assert query.user_message.content == expected_content
+        assert received_contents == [expected_content]
 
     @pytest.mark.asyncio
     async def test_adapter_without_stream_method_defaults_non_stream(self, fake_app, mock_event_ctx, set_runner):

tests/unit_tests/test_matrix_relogin.py

@@ -1,61 +1,89 @@
+"""Regression tests for the production Matrix ``!relogin`` command."""
+
 from __future__ import annotations
 
-"""Regression test for the Matrix ``!relogin`` command (``_handle_relogin_command``).
+from types import SimpleNamespace
+from unittest.mock import AsyncMock, Mock, call
 
-The old code placed ``logout_cmd = ...`` on a line after an unconditional
-``continue`` inside the ``if not bridge.login_command or not bridge.dm_room_id``
-branch. Because the ``continue`` always fired, ``logout_cmd`` was never assigned
-on the configured path, so any bridge with both ``login_command`` and
-``dm_room_id`` raised ``UnboundLocalError``. The fix moves the assignment above
-the ``if`` so it runs for configured bridges.
+import pytest
 
-These tests replicate the loop's control flow with a minimal fake bridge so they
-run without the Matrix SDK / langbot_plugin dependency.
-"""
+from langbot.pkg.platform.sources import matrix
 
 
-class FakeBridge:
-    def __init__(self, user_id: str, login_command: str, logout_command: str = '', dm_room_id: str | None = None):
-        self.user_id = user_id
-        self.login_command = login_command
-        self.logout_command = logout_command
-        self.dm_room_id = dm_room_id
+@pytest.fixture
+def bridge():
+    bridge = matrix.BridgeState('@u:example.org', 'login', '', '', '')
+    bridge.dm_room_id = '!bridge:example.org'
+    bridge.logged_in = True
+    return bridge
 
 
-def _relogin_commands(bridges: list[FakeBridge]) -> list[str]:
-    """Return the logout commands the fixed loop would send for each bridge."""
-    commands: list[str] = []
-    for bridge in bridges:
-        if not bridge.login_command or not bridge.dm_room_id:
-            continue
-        # Use configured logout command, fallback to deriving from login command.
-        logout_cmd = bridge.logout_command or bridge.login_command.replace('login', 'logout')
-        commands.append(logout_cmd)
-    return commands
+@pytest.fixture
+def adapter(monkeypatch):
+    adapter = matrix.MatrixAdapter.model_construct(
+        client=SimpleNamespace(room_send=AsyncMock()),
+        logger=None,
+    )
+    monkeypatch.setattr(matrix.asyncio, 'sleep', AsyncMock())
+    monkeypatch.setattr(matrix.MatrixAdapter, '_restart_bridge_login', Mock())
+    return adapter
 
 
-def test_configured_bridge_produces_logout_command_without_error() -> None:
-    bridges = [FakeBridge('@u:example.org', 'login', dm_room_id='!room:example.org')]
-    # Old code raised UnboundLocalError here; the fix must return the derived
-    # logout command (no configured logout_command -> derive from login).
-    assert _relogin_commands(bridges) == ['logout']
+async def test_configured_bridge_produces_logout_command_without_error(adapter, bridge) -> None:
+    adapter._bridges = [bridge]
+
+    await adapter._handle_relogin_command('!admin:example.org')
+
+    assert adapter.client.room_send.await_count == 2
+    assert adapter.client.room_send.await_args_list[0] == call(
+        room_id='!bridge:example.org',
+        message_type='m.room.message',
+        content={'msgtype': 'm.text', 'body': 'logout'},
+    )
+    adapter._restart_bridge_login.assert_called_once_with(bridge)
+    assert bridge.logged_in is False
 
 
-def test_configured_logout_command_is_used_verbatim() -> None:
-    bridges = [FakeBridge('@u:example.org', 'login', logout_command='leave', dm_room_id='!room:example.org')]
-    assert _relogin_commands(bridges) == ['leave']
+async def test_configured_logout_command_is_used_verbatim(adapter, bridge) -> None:
+    bridge.logout_command = 'leave'
+    adapter._bridges = [bridge]
+
+    await adapter._handle_relogin_command('!admin:example.org')
+
+    assert adapter.client.room_send.await_count == 2
+    assert adapter.client.room_send.await_args_list[0] == call(
+        room_id='!bridge:example.org',
+        message_type='m.room.message',
+        content={'msgtype': 'm.text', 'body': 'leave'},
+    )
+    adapter._restart_bridge_login.assert_called_once_with(bridge)
+    assert bridge.logged_in is False
 
 
-def test_skipped_bridge_is_ignored() -> None:
-    # Missing dm_room_id -> skipped, no command emitted.
-    bridges = [FakeBridge('@u:example.org', 'login', dm_room_id=None)]
-    assert _relogin_commands(bridges) == []
+async def test_skipped_bridge_is_ignored(adapter, bridge) -> None:
+    bridge.dm_room_id = None
+    adapter._bridges = [bridge]
+
+    await adapter._handle_relogin_command('!admin:example.org')
+
+    adapter.client.room_send.assert_awaited_once()
+    assert adapter.client.room_send.await_args.kwargs['room_id'] == '!admin:example.org'
+    adapter._restart_bridge_login.assert_not_called()
+    assert bridge.logged_in is True
 
 
-def test_relogin_never_raises_for_mixed_configurations() -> None:
-    bridges = [
-        FakeBridge('@skip:example.org', '', dm_room_id='!room:example.org'),  # no login_command
-        FakeBridge('@ok:example.org', 'login', dm_room_id='!room:example.org'),
-        FakeBridge('@skip2:example.org', 'login', dm_room_id=None),  # no dm_room_id
+async def test_relogin_never_raises_for_mixed_configurations(adapter, bridge) -> None:
+    missing_login = matrix.BridgeState('@skip:example.org', '', '', '', '')
+    missing_login.dm_room_id = '!skip:example.org'
+    missing_room = matrix.BridgeState('@skip2:example.org', 'login', '', '', '')
+    adapter._bridges = [missing_login, bridge, missing_room]
+
+    await adapter._handle_relogin_command('!admin:example.org')
+
+    assert [sent.kwargs['room_id'] for sent in adapter.client.room_send.await_args_list] == [
+        '!bridge:example.org',
+        '!admin:example.org',
     ]
-    assert _relogin_commands(bridges) == ['logout']
+    assert adapter.client.room_send.await_args_list[0].kwargs['content']['body'] == 'logout'
+    adapter._restart_bridge_login.assert_called_once_with(bridge)
+    assert bridge.logged_in is False

tests/unit_tests/vector/test_vdb_filter_conversion.py

@@ -10,6 +10,9 @@ from __future__ import annotations
 
 from importlib import import_module
 
+import pytest
+from sqlalchemy import create_engine, select, text
+
 
 def get_qdrant_module():
     """Lazy import qdrant module."""
@@ -261,6 +264,37 @@ class TestPgVectorFilterConversion:
     Tests use only these supported fields.
     """
 
+    @pytest.fixture
+    def matching_file_ids(self):
+        """Execute generated predicates against fixed rows, including the range boundary."""
+        pgvector_module = get_pgvector_module()
+        engine = create_engine('sqlite://')
+        try:
+            with engine.begin() as connection:
+                connection.execute(text('CREATE TABLE langbot_vectors (file_id TEXT, chunk_uuid TEXT, text TEXT)'))
+                connection.execute(
+                    text('INSERT INTO langbot_vectors VALUES (:file_id, :chunk_uuid, :text)'),
+                    [
+                        {'file_id': 'abc', 'chunk_uuid': 'uuid_099', 'text': 'keep'},
+                        {'file_id': 'deleted', 'chunk_uuid': 'uuid_100', 'text': ''},
+                        {'file_id': 'def', 'chunk_uuid': 'uuid_101', 'text': 'keep'},
+                        {'file_id': 'outside', 'chunk_uuid': 'uuid_102', 'text': 'keep'},
+                    ],
+                )
+
+                def matching(filter_dict):
+                    entry = pgvector_module.PgVectorEntry
+                    statement = (
+                        select(entry.file_id)
+                        .where(*pgvector_module._build_pg_conditions(filter_dict))
+                        .order_by(entry.chunk_uuid)
+                    )
+                    return connection.execute(statement).scalars().all()
+
+                yield matching
+        finally:
+            engine.dispose()
+
     def test_empty_filter_returns_empty_list(self):
         """Empty filter dict returns empty list."""
         pgvector_module = get_pgvector_module()
@@ -268,73 +302,40 @@ class TestPgVectorFilterConversion:
         result = pgvector_module._build_pg_conditions({})
         assert result == []
 
-    def test_eq_operator_creates_equality_condition(self):
+    def test_eq_operator_creates_equality_condition(self, matching_file_ids):
         """$eq operator creates SQLAlchemy == condition."""
-        pgvector_module = get_pgvector_module()
+        assert matching_file_ids({'file_id': 'abc'}) == ['abc']
 
-        result = pgvector_module._build_pg_conditions({'file_id': 'abc'})
-
-        assert len(result) == 1
-        # Verify it's a SQLAlchemy BinaryExpression
-        from sqlalchemy.sql.expression import BinaryExpression
-
-        assert isinstance(result[0], BinaryExpression)
-
-    def test_ne_operator_creates_inequality_condition(self):
+    def test_ne_operator_creates_inequality_condition(self, matching_file_ids):
         """$ne operator creates SQLAlchemy != condition."""
-        pgvector_module = get_pgvector_module()
+        assert matching_file_ids({'file_id': {'$ne': 'deleted'}}) == ['abc', 'def', 'outside']
 
-        result = pgvector_module._build_pg_conditions({'file_id': {'$ne': 'deleted'}})
-
-        assert len(result) == 1
-        # Operator should be ne (not equals)
-        assert '!=' in str(result[0]) or 'ne' in str(result[0].operator)
-
-    def test_comparison_operators(self):
+    def test_comparison_operators(self, matching_file_ids):
         """$gt, $gte, $lt, $lte create comparison conditions."""
-        pgvector_module = get_pgvector_module()
-
-        # Test all comparison operators with supported field
-        for op, expected_op in [
-            ('$gt', '>'),
-            ('$gte', '>='),
-            ('$lt', '<'),
-            ('$lte', '<='),
+        for op, expected_ids in [
+            ('$gt', ['def', 'outside']),
+            ('$gte', ['deleted', 'def', 'outside']),
+            ('$lt', ['abc']),
+            ('$lte', ['abc', 'deleted']),
         ]:
-            result = pgvector_module._build_pg_conditions({'chunk_uuid': {op: 'uuid_100'}})
-            assert len(result) == 1
-            assert expected_op in str(result[0])
+            assert matching_file_ids({'chunk_uuid': {op: 'uuid_100'}}) == expected_ids
 
-    def test_in_operator_creates_in_condition(self):
+    def test_in_operator_creates_in_condition(self, matching_file_ids):
         """$in operator creates SQLAlchemy in_ condition."""
-        pgvector_module = get_pgvector_module()
+        assert matching_file_ids({'file_id': {'$in': ['abc', 'deleted']}}) == ['abc', 'deleted']
 
-        result = pgvector_module._build_pg_conditions({'file_id': {'$in': ['a', 'b', 'c']}})
-
-        assert len(result) == 1
-        assert 'IN' in str(result[0]).upper()
-
-    def test_nin_operator_creates_notin_condition(self):
+    def test_nin_operator_creates_notin_condition(self, matching_file_ids):
         """$nin operator creates SQLAlchemy notin_ condition."""
-        pgvector_module = get_pgvector_module()
+        assert matching_file_ids({'file_id': {'$nin': ['abc', 'deleted']}}) == ['def', 'outside']
 
-        result = pgvector_module._build_pg_conditions({'file_id': {'$nin': ['a', 'b']}})
-
-        assert len(result) == 1
-        assert 'NOT IN' in str(result[0]).upper()
-
-    def test_multiple_conditions_list(self):
+    def test_multiple_conditions_list(self, matching_file_ids):
         """Multiple conditions return list of conditions."""
-        pgvector_module = get_pgvector_module()
-
-        result = pgvector_module._build_pg_conditions(
+        assert matching_file_ids(
             {
-                'file_id': 'abc',
-                'chunk_uuid': {'$ne': 'def'},
+                'file_id': {'$in': ['abc', 'deleted']},
+                'chunk_uuid': {'$ne': 'uuid_100'},
             }
-        )
-
-        assert len(result) == 2
+        ) == ['abc']
 
     def test_unsupported_field_dropped(self):
         """Unsupported fields are dropped (not in _PG_SUPPORTED_FIELDS)."""
@@ -343,27 +344,17 @@ class TestPgVectorFilterConversion:
         result = pgvector_module._build_pg_conditions({'unknown_field': 'value'})
         assert result == []
 
-    def test_uuid_alias_resolved(self):
+    def test_uuid_alias_resolved(self, matching_file_ids):
         """'uuid' alias is resolved to 'chunk_uuid'."""
-        pgvector_module = get_pgvector_module()
+        assert matching_file_ids({'uuid': 'uuid_100'}) == ['deleted']
 
-        result = pgvector_module._build_pg_conditions({'uuid': 'abc'})
-
-        assert len(result) == 1
-        # Should reference chunk_uuid column
-        assert 'chunk_uuid' in str(result[0])
-
-    def test_supported_fields_only(self):
+    def test_supported_fields_only(self, matching_file_ids):
         """Only supported fields (text, file_id, chunk_uuid) are kept."""
-        pgvector_module = get_pgvector_module()
-
-        result = pgvector_module._build_pg_conditions(
+        assert matching_file_ids(
             {
                 'text': {'$ne': ''},
-                'file_id': 'abc',
-                'chunk_uuid': {'$in': ['x', 'y']},
+                'file_id': {'$in': ['abc', 'deleted', 'outside']},
+                'chunk_uuid': {'$in': ['uuid_099', 'uuid_100', 'uuid_101']},
                 'unsupported': 'value',
             }
-        )
-
-        assert len(result) == 3  # Only supported fields
+        ) == ['abc']