-
Notifications
You must be signed in to change notification settings - Fork 99
LCORE-1574: Integration tests for conversation compaction #2427
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| """Shared fixtures for integration tests.""" | ||
|
|
||
| import os | ||
| from collections import defaultdict | ||
| from collections.abc import AsyncIterator, Generator | ||
| from pathlib import Path | ||
| from typing import Any, Optional | ||
|
|
@@ -10,6 +11,9 @@ | |
| from fastapi.testclient import TestClient | ||
| from ogx_api.openai_responses import OpenAIResponseObject | ||
| from ogx_client.types import ListModelsResponse, VersionInfo | ||
| from ogx_client.types.conversations.item_list_response import ( | ||
| OpenAIResponseMessageOutput, | ||
| ) | ||
| from ogx_client.types.model import Model | ||
| from pydantic_ai import AgentRunResultEvent | ||
| from pydantic_ai.messages import ( | ||
|
|
@@ -355,6 +359,7 @@ def mock_agent_run_stream(events: list[Any]) -> Any: | |
| """Build an async context manager that yields pydantic-ai stream events.""" | ||
|
|
||
| async def _event_stream() -> AsyncIterator[Any]: | ||
| """Yield pre-built events one at a time.""" | ||
| for event in events: | ||
| yield event | ||
|
|
||
|
|
@@ -441,6 +446,86 @@ def set_streaming_query_agent_run( | |
| ) | ||
|
|
||
|
|
||
| # ========================================== | ||
| # In-Memory Conversation Store | ||
| # ========================================== | ||
|
|
||
|
|
||
| class InMemoryConversationStore: | ||
| """In-memory fake for the Llama Stack conversations.items API. | ||
|
|
||
| Provides stateful ``list`` and ``create`` methods that can be wired onto a | ||
| mock ``AsyncOgxClient`` so that ``get_all_conversation_items`` and | ||
| ``_write_summary_marker`` (and ``append_turn_items_to_conversation``) work | ||
| without patching. | ||
| """ | ||
|
|
||
| def __init__(self) -> None: | ||
| """Initialize an empty in-memory conversation store.""" | ||
| self._store: dict[str, list[OpenAIResponseMessageOutput]] = defaultdict(list) | ||
|
|
||
| @property | ||
| def store(self) -> dict[str, list[OpenAIResponseMessageOutput]]: | ||
| """Direct access to the backing store for seeding data in tests.""" | ||
| return self._store | ||
|
|
||
| def _dict_to_message(self, raw: dict[str, Any]) -> OpenAIResponseMessageOutput: | ||
| """Convert a raw item dict (as sent by production code) to a typed object.""" | ||
| content = raw.get("content", "") | ||
| if isinstance(content, list): | ||
| content = " ".join( | ||
| part.get("text", "") for part in content if isinstance(part, dict) | ||
| ) | ||
| role = raw.get("role", "user") | ||
| return OpenAIResponseMessageOutput.model_construct( | ||
| type="message", | ||
| role=role, | ||
| content=content, | ||
| ) | ||
|
|
||
| async def create(self, conversation_id: str, *, items: Any, **_kwargs: Any) -> Any: | ||
| """Async fake for ``client.conversations.items.create``.""" | ||
| for raw in items: | ||
| if isinstance(raw, dict): | ||
| self._store[conversation_id].append(self._dict_to_message(raw)) | ||
| else: | ||
| self._store[conversation_id].append(raw) | ||
|
|
||
| def list(self, conversation_id: str, **_kwargs: Any) -> "_FakePaginator": | ||
| """Fake for ``client.conversations.items.list`` (returns an awaitable).""" | ||
| return _FakePaginator(list(self._store.get(conversation_id, []))) | ||
|
|
||
|
|
||
| class _FakePage: | ||
| """Single-page result matching the ``AsyncOpenAICursorPage`` protocol.""" | ||
|
|
||
| def __init__(self, data: list[OpenAIResponseMessageOutput]) -> None: | ||
| self.data = data | ||
|
|
||
| def has_next_page(self) -> bool: | ||
| """Return False — the fake always returns all items in one page.""" | ||
| return False | ||
|
|
||
| async def get_next_page(self) -> "_FakePage": | ||
| """Return an empty page (should never be called).""" | ||
| return _FakePage([]) | ||
|
|
||
|
|
||
| class _FakePaginator: # pylint: disable=too-few-public-methods | ||
| """Awaitable object matching the ``AsyncPaginator`` protocol.""" | ||
|
|
||
| def __init__(self, data: list[OpenAIResponseMessageOutput]) -> None: | ||
| self._page = _FakePage(data) | ||
|
|
||
| def __await__(self) -> Any: | ||
| """Allow ``await paginator`` to return the page.""" | ||
| return self._resolve().__await__() # pylint: disable=no-member | ||
|
|
||
| async def _resolve(self) -> _FakePage: | ||
| """Return the single pre-built page.""" | ||
| return self._page | ||
|
|
||
|
|
||
| # ========================================== | ||
| # Fixtures | ||
| # ========================================== | ||
|
|
@@ -746,9 +831,11 @@ def mock_ogx_client_fixture( | |
| defaults for integration tests. Individual tests can override specific | ||
| behaviors as needed. | ||
|
|
||
| Patches AsyncOgxClientHolder in both app.endpoints.query and app.main | ||
| to ensure the mock is active during TestClient startup (when app.main imports | ||
| and initializes the client) and during endpoint execution. | ||
| Patches AsyncOgxClientHolder in app.endpoints.query, app.main, | ||
| app.endpoints.a2a, app.endpoints.responses, utils.endpoints, and | ||
| app.endpoints.streaming_query to ensure the mock is active during | ||
| TestClient startup (when app.main imports and initializes the | ||
| client) and during endpoint execution. | ||
|
|
||
| Args: | ||
| mocker: pytest-mock fixture used to create and patch mocks. | ||
|
|
@@ -764,6 +851,12 @@ def mock_ogx_client_fixture( | |
| mocker.patch( | ||
| "app.endpoints.conversations_v1.AsyncOgxClientHolder", mock_holder_class | ||
| ) | ||
| mocker.patch("app.endpoints.a2a.AsyncOgxClientHolder", mock_holder_class) | ||
| mocker.patch("app.endpoints.responses.AsyncOgxClientHolder", mock_holder_class) | ||
| mocker.patch("utils.endpoints.AsyncOgxClientHolder", mock_holder_class) | ||
| mocker.patch( | ||
| "app.endpoints.streaming_query.AsyncOgxClientHolder", mock_holder_class | ||
| ) | ||
|
|
||
| mock_client = mocker.AsyncMock() | ||
|
|
||
|
|
@@ -827,6 +920,29 @@ def mock_ogx_client_fixture( | |
| yield mock_client | ||
|
|
||
|
|
||
| @pytest.fixture(name="mock_conversation_store") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| def conversation_store_fixture( | ||
| mock_ogx_client: Any, | ||
| ) -> InMemoryConversationStore: | ||
| """Wire an in-memory conversation store into the mock Llama Stack client. | ||
|
|
||
| Replaces ``conversations.items.list`` and ``conversations.items.create`` | ||
| on the mock client with stateful fakes so that conversation items persist | ||
| across calls within a single test. Use ``await store.create(conv_id, items=...)`` | ||
| to pre-populate a conversation. | ||
|
|
||
| Args: | ||
| mock_ogx_client: The mocked Llama Stack client from mock_ogx_client_fixture. | ||
|
|
||
| Returns: | ||
| The InMemoryConversationStore instance backing the mock client. | ||
| """ | ||
| store = InMemoryConversationStore() | ||
| mock_ogx_client.conversations.items.list = store.list | ||
| mock_ogx_client.conversations.items.create = store.create | ||
| return store | ||
|
|
||
|
|
||
| @pytest.fixture(name="mock_query_agent") | ||
| def mock_query_agent_fixture(mocker: MockerFixture) -> Any: | ||
| """Patch build_agent for /query and return the mock agent.""" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docstring says the store exists so that
_write_summary_markerworks "without patching", but_patch_write_summary_marker(test file, line 89) patches it in every test, and the fake variant rebuilds the marker item by hand:That duplicates the construction at
src/utils/conversation_compaction.py:273-289, so a change to the real marker format would leave these tests passing. The fake store'screate(conversation_id, *, items, **kwargs)signature matches the real call site, so the patch can be dropped entirely - asserting on the store contents is both simpler and a stronger check thanmock_write_marker.assert_awaited_once().