From 74682eff0c2cf77dae4cd7e436c58c0ecde97a1c Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Mon, 23 Dec 2024 13:12:42 -0700 Subject: [PATCH 01/56] Conversation endpoint --- openhands/server/routes/conversation.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/openhands/server/routes/conversation.py b/openhands/server/routes/conversation.py index 690eb0673a25..448746959775 100644 --- a/openhands/server/routes/conversation.py +++ b/openhands/server/routes/conversation.py @@ -3,6 +3,7 @@ from openhands.core.logger import openhands_logger as logger from openhands.runtime.base import Runtime +from openhands.server.shared import session_manager app = APIRouter(prefix='/api/conversations/{conversation_id}') @@ -104,3 +105,12 @@ async def search_events( 'events': matching_events, 'has_more': has_more, } + + +@app.get('/conversation/search') +async def search_conversations(request: Request): + conversations = session_manager.file_store.list('sessions') + return { + 'conversations': conversations, + 'has_more': False, + } From dc820946b7d6d0b0ac89e7a0ccf1fee0b3178865 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Mon, 23 Dec 2024 14:26:58 -0700 Subject: [PATCH 02/56] Added endpoint for conversation search --- openhands/server/routes/conversation.py | 10 ------- openhands/server/routes/new_conversation.py | 32 +++++++++++++++++++++ 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/openhands/server/routes/conversation.py b/openhands/server/routes/conversation.py index 448746959775..690eb0673a25 100644 --- a/openhands/server/routes/conversation.py +++ b/openhands/server/routes/conversation.py @@ -3,7 +3,6 @@ from openhands.core.logger import openhands_logger as logger from openhands.runtime.base import Runtime -from openhands.server.shared import session_manager app = APIRouter(prefix='/api/conversations/{conversation_id}') @@ -105,12 +104,3 @@ async def search_events( 'events': matching_events, 'has_more': has_more, } - - -@app.get('/conversation/search') -async def search_conversations(request: Request): - conversations = session_manager.file_store.list('sessions') - return { - 'conversations': conversations, - 'has_more': False, - } diff --git a/openhands/server/routes/new_conversation.py b/openhands/server/routes/new_conversation.py index 8dd75eb63249..9eb8687a4587 100644 --- a/openhands/server/routes/new_conversation.py +++ b/openhands/server/routes/new_conversation.py @@ -1,3 +1,4 @@ +import json import uuid from fastapi import APIRouter, Request @@ -74,3 +75,34 @@ async def new_conversation(request: Request, data: InitSessionRequest): conversation_id, conversation_init_data ) return JSONResponse(content={'status': 'ok', 'conversation_id': conversation_id}) + + +@app.get('/conversations') +async def search_conversations(request: Request): + file_store = session_manager.file_store + conversations = [] + try: + for path in file_store.list('sessions/') or []: + try: + session_id = path.split('/')[1] + if not session_id.startswith('.'): + events = file_store.list(f'{path}events/') + events = sorted(events) + event_path = events[-1] + event = json.loads(file_store.read(event_path)) + conversations.append( + { + 'id': session_id, + 'last_accessed_at': event.get('timestamp'), + } + ) + except Exception: # type: ignore + logger.warning( + f'Error loading path: {path}', exc_info=True, stack_info=True + ) + except Exception: # type: ignore + logger.warning('Error loading conversation', exc_info=True, stack_info=True) + return { + 'conversations': conversations, + 'has_more': False, + } From e3f9b7e3f3b9816420ede53b0628d586d436f3b8 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Mon, 23 Dec 2024 14:36:03 -0700 Subject: [PATCH 03/56] WIP --- openhands/server/routes/new_conversation.py | 3 +++ openhands/server/session/manager.py | 16 ++++++++-------- tests/unit/test_manager.py | 14 +++++++------- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/openhands/server/routes/new_conversation.py b/openhands/server/routes/new_conversation.py index 9eb8687a4587..820b50a62d7f 100644 --- a/openhands/server/routes/new_conversation.py +++ b/openhands/server/routes/new_conversation.py @@ -94,6 +94,9 @@ async def search_conversations(request: Request): { 'id': session_id, 'last_accessed_at': event.get('timestamp'), + 'status': 'RUNNING' + if session_manager.is_agent_loop_running(session_id) + else 'STOPPED', } ) except Exception: # type: ignore diff --git a/openhands/server/session/manager.py b/openhands/server/session/manager.py index 7b38c3b73828..9591400119c3 100644 --- a/openhands/server/session/manager.py +++ b/openhands/server/session/manager.py @@ -234,19 +234,19 @@ async def _cleanup_detached_conversations(self): logger.warning('error_cleaning_detached_conversations', exc_info=True) await asyncio.sleep(_CLEANUP_EXCEPTION_WAIT_TIME) - async def _is_agent_loop_running(self, sid: str) -> bool: - if await self._is_agent_loop_running_locally(sid): + async def is_agent_loop_running(self, sid: str) -> bool: + if await self.is_agent_loop_running_locally(sid): return True - if await self._is_agent_loop_running_in_cluster(sid): + if await self.is_agent_loop_running_in_cluster(sid): return True return False - async def _is_agent_loop_running_locally(self, sid: str) -> bool: + async def is_agent_loop_running_locally(self, sid: str) -> bool: if self._local_agent_loops_by_sid.get(sid, None): return True return False - async def _is_agent_loop_running_in_cluster(self, sid: str) -> bool: + async def is_agent_loop_running_in_cluster(self, sid: str) -> bool: """As the rest of the cluster if a session is running. Wait a for a short timeout for a reply""" redis_client = self._get_redis_client() if not redis_client: @@ -307,7 +307,7 @@ async def maybe_start_agent_loop( ) -> EventStream: logger.info(f'maybe_start_agent_loop:{sid}') session: Session | None = None - if not await self._is_agent_loop_running(sid): + if not await self.is_agent_loop_running(sid): logger.info(f'start_agent_loop:{sid}') session = Session( sid=sid, file_store=self.file_store, config=self.config, sio=self.sio @@ -328,7 +328,7 @@ async def _get_event_stream(self, sid: str) -> EventStream | None: logger.info(f'found_local_agent_loop:{sid}') return session.agent_session.event_stream - if await self._is_agent_loop_running_in_cluster(sid): + if await self.is_agent_loop_running_in_cluster(sid): logger.info(f'found_remote_agent_loop:{sid}') return EventStream(sid, self.file_store) @@ -352,7 +352,7 @@ async def send_to_event_stream(self, connection_id: str, data: dict): next_alive_check = last_alive_at + _CHECK_ALIVE_INTERVAL if ( next_alive_check > time.time() - or await self._is_agent_loop_running_in_cluster(sid) + or await self.is_agent_loop_running_in_cluster(sid) ): # Send the event to the other pod await redis_client.publish( diff --git a/tests/unit/test_manager.py b/tests/unit/test_manager.py index 7b16915df8f4..b4628dfdef31 100644 --- a/tests/unit/test_manager.py +++ b/tests/unit/test_manager.py @@ -41,7 +41,7 @@ async def test_session_not_running_in_cluster(): async with SessionManager( sio, AppConfig(), InMemoryFileStore() ) as session_manager: - result = await session_manager._is_agent_loop_running_in_cluster( + result = await session_manager.is_agent_loop_running_in_cluster( 'non-existant-session' ) assert result is False @@ -65,7 +65,7 @@ async def test_session_is_running_in_cluster(): async with SessionManager( sio, AppConfig(), InMemoryFileStore() ) as session_manager: - result = await session_manager._is_agent_loop_running_in_cluster( + result = await session_manager.is_agent_loop_running_in_cluster( 'existing-session' ) assert result is True @@ -93,7 +93,7 @@ async def test_init_new_local_session(): AsyncMock(), ), patch( - 'openhands.server.session.manager.SessionManager._is_agent_loop_running_in_cluster', + 'openhands.server.session.manager.SessionManager.is_agent_loop_running_in_cluster', is_agent_loop_running_in_cluster_mock, ), ): @@ -125,7 +125,7 @@ async def test_join_local_session(): AsyncMock(), ), patch( - 'openhands.server.session.manager.SessionManager._is_agent_loop_running_in_cluster', + 'openhands.server.session.manager.SessionManager.is_agent_loop_running_in_cluster', is_agent_loop_running_in_cluster_mock, ), ): @@ -158,7 +158,7 @@ async def test_join_cluster_session(): AsyncMock(), ), patch( - 'openhands.server.session.manager.SessionManager._is_agent_loop_running_in_cluster', + 'openhands.server.session.manager.SessionManager.is_agent_loop_running_in_cluster', is_agent_loop_running_in_cluster_mock, ), ): @@ -187,7 +187,7 @@ async def test_add_to_local_event_stream(): AsyncMock(), ), patch( - 'openhands.server.session.manager.SessionManager._is_agent_loop_running_in_cluster', + 'openhands.server.session.manager.SessionManager.is_agent_loop_running_in_cluster', is_agent_loop_running_in_cluster_mock, ), ): @@ -221,7 +221,7 @@ async def test_add_to_cluster_event_stream(): AsyncMock(), ), patch( - 'openhands.server.session.manager.SessionManager._is_agent_loop_running_in_cluster', + 'openhands.server.session.manager.SessionManager.is_agent_loop_running_in_cluster', is_agent_loop_running_in_cluster_mock, ), ): From 3ac818b59cae188754d63319cc3e99a26ff11e82 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Mon, 23 Dec 2024 14:53:40 -0700 Subject: [PATCH 04/56] WIP --- openhands/server/routes/new_conversation.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/openhands/server/routes/new_conversation.py b/openhands/server/routes/new_conversation.py index 820b50a62d7f..899e74b1f8e1 100644 --- a/openhands/server/routes/new_conversation.py +++ b/openhands/server/routes/new_conversation.py @@ -90,13 +90,13 @@ async def search_conversations(request: Request): events = sorted(events) event_path = events[-1] event = json.loads(file_store.read(event_path)) + running = await session_manager.is_agent_loop_running(session_id) conversations.append( { 'id': session_id, - 'last_accessed_at': event.get('timestamp'), - 'status': 'RUNNING' - if session_manager.is_agent_loop_running(session_id) - else 'STOPPED', + 'title': 'TODO', + 'updated_at': event.get('timestamp'), + 'status': 'RUNNING' if running else 'STOPPED', } ) except Exception: # type: ignore From 76f359c4ec5e38d396e601e7a2255cbe6e46a0a3 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Mon, 23 Dec 2024 15:17:27 -0700 Subject: [PATCH 05/56] Added specific types --- .../server/data_models/conversation_info.py | 12 +++++++++ .../data_models/conversation_result_set.py | 9 +++++++ .../server/data_models/conversation_status.py | 6 +++++ openhands/server/routes/new_conversation.py | 27 +++++++++++-------- 4 files changed, 43 insertions(+), 11 deletions(-) create mode 100644 openhands/server/data_models/conversation_info.py create mode 100644 openhands/server/data_models/conversation_result_set.py create mode 100644 openhands/server/data_models/conversation_status.py diff --git a/openhands/server/data_models/conversation_info.py b/openhands/server/data_models/conversation_info.py new file mode 100644 index 000000000000..7e6900c8ae73 --- /dev/null +++ b/openhands/server/data_models/conversation_info.py @@ -0,0 +1,12 @@ +from dataclasses import dataclass +from datetime import datetime + +from openhands.server.data_models.conversation_status import ConversationStatus + + +@dataclass +class ConversationInfo: + id: str + title: str | None = None + last_updated_at: datetime | None = None + status: ConversationStatus = ConversationStatus.STOPPED diff --git a/openhands/server/data_models/conversation_result_set.py b/openhands/server/data_models/conversation_result_set.py new file mode 100644 index 000000000000..d6a203b0062a --- /dev/null +++ b/openhands/server/data_models/conversation_result_set.py @@ -0,0 +1,9 @@ +from dataclasses import dataclass, field + +from openhands.server.data_models.conversation_info import ConversationInfo + + +@dataclass +class ConversationResultSet: + results: list[ConversationInfo] = field(default_factory=list) + next_page_id: str | None = None diff --git a/openhands/server/data_models/conversation_status.py b/openhands/server/data_models/conversation_status.py new file mode 100644 index 000000000000..8d476fa4e3a5 --- /dev/null +++ b/openhands/server/data_models/conversation_status.py @@ -0,0 +1,6 @@ +from enum import Enum + + +class ConversationStatus(Enum): + RUNNING = 'RUNNING' + STOPPED = 'STOPPED' diff --git a/openhands/server/routes/new_conversation.py b/openhands/server/routes/new_conversation.py index 899e74b1f8e1..b069cd28b8d3 100644 --- a/openhands/server/routes/new_conversation.py +++ b/openhands/server/routes/new_conversation.py @@ -1,5 +1,6 @@ import json import uuid +from datetime import datetime from fastapi import APIRouter, Request from fastapi.responses import JSONResponse @@ -7,6 +8,9 @@ from pydantic import BaseModel from openhands.core.logger import openhands_logger as logger +from openhands.server.data_models.conversation_info import ConversationInfo +from openhands.server.data_models.conversation_result_set import ConversationResultSet +from openhands.server.data_models.conversation_status import ConversationStatus from openhands.server.routes.settings import SettingsStoreImpl from openhands.server.session.conversation_init_data import ConversationInitData from openhands.server.shared import config, session_manager @@ -78,7 +82,7 @@ async def new_conversation(request: Request, data: InitSessionRequest): @app.get('/conversations') -async def search_conversations(request: Request): +async def search_conversations(request: Request) -> ConversationResultSet: file_store = session_manager.file_store conversations = [] try: @@ -92,12 +96,16 @@ async def search_conversations(request: Request): event = json.loads(file_store.read(event_path)) running = await session_manager.is_agent_loop_running(session_id) conversations.append( - { - 'id': session_id, - 'title': 'TODO', - 'updated_at': event.get('timestamp'), - 'status': 'RUNNING' if running else 'STOPPED', - } + ConversationInfo( + id=session_id, + title='TODO', + last_updated_at=datetime.fromisoformat( + event.get('timestamp') + ), + status=ConversationStatus.RUNNING + if running + else ConversationStatus.STOPPED, + ) ) except Exception: # type: ignore logger.warning( @@ -105,7 +113,4 @@ async def search_conversations(request: Request): ) except Exception: # type: ignore logger.warning('Error loading conversation', exc_info=True, stack_info=True) - return { - 'conversations': conversations, - 'has_more': False, - } + return ConversationResultSet(results=conversations, next_page_id=None) From fe0acad1f2f1a3c153f4a2b0688a080a31e3a07c Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Thu, 26 Dec 2024 18:14:11 -0700 Subject: [PATCH 06/56] Metadata --- openhands/server/routes/new_conversation.py | 44 ++++++++++++++++----- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/openhands/server/routes/new_conversation.py b/openhands/server/routes/new_conversation.py index b069cd28b8d3..914272106622 100644 --- a/openhands/server/routes/new_conversation.py +++ b/openhands/server/routes/new_conversation.py @@ -1,3 +1,4 @@ +import base64 import json import uuid from datetime import datetime @@ -82,35 +83,60 @@ async def new_conversation(request: Request, data: InitSessionRequest): @app.get('/conversations') -async def search_conversations(request: Request) -> ConversationResultSet: +async def search_conversations( + request: Request, + page_id: str | None = None, + limit: int = 20, +) -> ConversationResultSet: file_store = session_manager.file_store conversations = [] try: - for path in file_store.list('sessions/') or []: + session_ids = [ + path.split('/')[1] + for path in file_store.list('sessions/') + if not path.startswith('sessions/.') + ] + start = 0 + if page_id: + start = int(base64.b64decode(page_id.encode()).decode()) + end = limit + start + next_page_id = None + if len(session_ids) > end: + next_page_id = base64.b64encode(str(end).encode()).decode() + else: + end = len(session_ids) + running_sessions = await session_manager.get_agent_loop_running( + set(session_ids[start:end]) + ) + for session_id in session_ids: try: - session_id = path.split('/')[1] if not session_id.startswith('.'): - events = file_store.list(f'{path}events/') + metadata = json.loads( + file_store.read(f'sessions/{session_id}/metadata.json') + ) + title = metadata.get('title', '') + events = file_store.list(f'sessions/{session_id}/events/') events = sorted(events) event_path = events[-1] event = json.loads(file_store.read(event_path)) - running = await session_manager.is_agent_loop_running(session_id) conversations.append( ConversationInfo( id=session_id, - title='TODO', + title=title, last_updated_at=datetime.fromisoformat( event.get('timestamp') ), status=ConversationStatus.RUNNING - if running + if session_id in running_sessions else ConversationStatus.STOPPED, ) ) except Exception: # type: ignore logger.warning( - f'Error loading path: {path}', exc_info=True, stack_info=True + f'Error loading session: {session_id}', + exc_info=True, + stack_info=True, ) except Exception: # type: ignore logger.warning('Error loading conversation', exc_info=True, stack_info=True) - return ConversationResultSet(results=conversations, next_page_id=None) + return ConversationResultSet(results=conversations, next_page_id=next_page_id) From 5edc93d90ea82fa5010263a7cb26aab924bff580 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Fri, 27 Dec 2024 09:33:57 -0700 Subject: [PATCH 07/56] Search conversations endpoint --- frontend/public/mockServiceWorker.js | 18 +++- openhands/server/routes/new_conversation.py | 98 ++++++++++----------- openhands/utils/search_utils.py | 14 +++ 3 files changed, 77 insertions(+), 53 deletions(-) create mode 100644 openhands/utils/search_utils.py diff --git a/frontend/public/mockServiceWorker.js b/frontend/public/mockServiceWorker.js index fead0b3ff91c..ec47a9a50a24 100644 --- a/frontend/public/mockServiceWorker.js +++ b/frontend/public/mockServiceWorker.js @@ -8,8 +8,8 @@ * - Please do NOT serve this file on production. */ -const PACKAGE_VERSION = '2.6.6' -const INTEGRITY_CHECKSUM = 'ca7800994cc8bfb5eb961e037c877074' +const PACKAGE_VERSION = '2.7.0' +const INTEGRITY_CHECKSUM = '00729d72e3b82faf54ca8b9621dbb96f' const IS_MOCKED_RESPONSE = Symbol('isMockedResponse') const activeClientIds = new Set() @@ -199,7 +199,19 @@ async function getResponse(event, client, requestId) { // Remove the "accept" header value that marked this request as passthrough. // This prevents request alteration and also keeps it compliant with the // user-defined CORS policies. - headers.delete('accept', 'msw/passthrough') + const acceptHeader = headers.get('accept') + if (acceptHeader) { + const values = acceptHeader.split(',').map((value) => value.trim()) + const filteredValues = values.filter( + (value) => value !== 'msw/passthrough', + ) + + if (filteredValues.length > 0) { + headers.set('accept', filteredValues.join(', ')) + } else { + headers.delete('accept') + } + } return fetch(requestClone, { headers }) } diff --git a/openhands/server/routes/new_conversation.py b/openhands/server/routes/new_conversation.py index 914272106622..c0d64f641183 100644 --- a/openhands/server/routes/new_conversation.py +++ b/openhands/server/routes/new_conversation.py @@ -19,7 +19,9 @@ ConversationMetadata, ConversationStore, ) +from openhands.storage.files import FileStore from openhands.utils.async_utils import call_sync_from_async +from openhands.utils.search_utils import offset_to_page_id, page_id_to_offset app = APIRouter(prefix='/api') @@ -84,59 +86,55 @@ async def new_conversation(request: Request, data: InitSessionRequest): @app.get('/conversations') async def search_conversations( - request: Request, page_id: str | None = None, limit: int = 20, ) -> ConversationResultSet: file_store = session_manager.file_store conversations = [] - try: - session_ids = [ - path.split('/')[1] - for path in file_store.list('sessions/') - if not path.startswith('sessions/.') - ] - start = 0 - if page_id: - start = int(base64.b64decode(page_id.encode()).decode()) - end = limit + start - next_page_id = None - if len(session_ids) > end: - next_page_id = base64.b64encode(str(end).encode()).decode() - else: - end = len(session_ids) - running_sessions = await session_manager.get_agent_loop_running( - set(session_ids[start:end]) - ) - for session_id in session_ids: - try: - if not session_id.startswith('.'): - metadata = json.loads( - file_store.read(f'sessions/{session_id}/metadata.json') - ) - title = metadata.get('title', '') - events = file_store.list(f'sessions/{session_id}/events/') - events = sorted(events) - event_path = events[-1] - event = json.loads(file_store.read(event_path)) - conversations.append( - ConversationInfo( - id=session_id, - title=title, - last_updated_at=datetime.fromisoformat( - event.get('timestamp') - ), - status=ConversationStatus.RUNNING - if session_id in running_sessions - else ConversationStatus.STOPPED, - ) - ) - except Exception: # type: ignore - logger.warning( - f'Error loading session: {session_id}', - exc_info=True, - stack_info=True, - ) - except Exception: # type: ignore - logger.warning('Error loading conversation', exc_info=True, stack_info=True) + session_ids = [ + path.split('/')[1] + for path in file_store.list('sessions/') + if not path.startswith('sessions/.') + ] + num_sessions = len(session_ids) + start = page_id_to_offset(page_id) + end = min(limit + start, num_sessions) + next_page_id = offset_to_page_id(end, end < num_sessions) + running_sessions = await session_manager.get_agent_loop_running( + set(session_ids[start:end]) + ) + for session_id in session_ids: + try: + is_running = session_id in running_sessions + conversation_info = _get_conversation_info(session_id, is_running, file_store) + if conversation_info: + conversations.append(conversation_info) + except Exception: # type: ignore + # If a conversation is corrupt, we simply log and skip. + logger.warning( + f'Error loading session: {session_id}', + exc_info=True, + stack_info=True, + ) return ConversationResultSet(results=conversations, next_page_id=next_page_id) + + +def _get_conversation_info(session_id: str, is_running: bool, file_store: FileStore) -> ConversationInfo: + metadata = json.loads( + file_store.read(f'sessions/{session_id}/metadata.json') + ) + title = metadata.get('title', '') + events = file_store.list(f'sessions/{session_id}/events/') + events = sorted(events) + event_path = events[-1] + event = json.loads(file_store.read(event_path)) + return ConversationInfo( + id=session_id, + title=title, + last_updated_at=datetime.fromisoformat( + event.get('timestamp') + ), + status=ConversationStatus.RUNNING + if is_running + else ConversationStatus.STOPPED, + ) diff --git a/openhands/utils/search_utils.py b/openhands/utils/search_utils.py new file mode 100644 index 000000000000..acde907ed982 --- /dev/null +++ b/openhands/utils/search_utils.py @@ -0,0 +1,14 @@ +import base64 + + +def offset_to_page_id(offset: int, has_next: bool) -> str | None: + if not has_next: + return None + next_page_id = base64.b64encode(str(offset).encode()).decode() + return next_page_id + + +def page_id_to_offset(page_id: str | None) -> int: + if not page_id: + return 0 + \ No newline at end of file From 8781657a549e6f626fd0c3f14d9ba77b82efec89 Mon Sep 17 00:00:00 2001 From: openhands Date: Fri, 27 Dec 2024 16:44:20 +0000 Subject: [PATCH 08/56] Add unit tests for search_utils and implement page_id_to_offset --- openhands/utils/search_utils.py | 3 ++- tests/unit/test_search_utils.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 tests/unit/test_search_utils.py diff --git a/openhands/utils/search_utils.py b/openhands/utils/search_utils.py index acde907ed982..5b3fd6e9c6e9 100644 --- a/openhands/utils/search_utils.py +++ b/openhands/utils/search_utils.py @@ -11,4 +11,5 @@ def offset_to_page_id(offset: int, has_next: bool) -> str | None: def page_id_to_offset(page_id: str | None) -> int: if not page_id: return 0 - \ No newline at end of file + offset = int(base64.b64decode(page_id).decode()) + return offset \ No newline at end of file diff --git a/tests/unit/test_search_utils.py b/tests/unit/test_search_utils.py new file mode 100644 index 000000000000..7216a9b75e1c --- /dev/null +++ b/tests/unit/test_search_utils.py @@ -0,0 +1,31 @@ +import pytest +from openhands.utils.search_utils import offset_to_page_id, page_id_to_offset + + +def test_offset_to_page_id(): + # Test with has_next=True + assert offset_to_page_id(10, True) == "MTA=" # base64 encoding of "10" + assert offset_to_page_id(0, True) == "MA==" # base64 encoding of "0" + assert offset_to_page_id(100, True) == "MTAw" # base64 encoding of "100" + + # Test with has_next=False should return None + assert offset_to_page_id(10, False) is None + assert offset_to_page_id(0, False) is None + + +def test_page_id_to_offset(): + # Test with valid page_ids + assert page_id_to_offset("MTA=") == 10 # base64 decoding of "10" + assert page_id_to_offset("MA==") == 0 # base64 decoding of "0" + assert page_id_to_offset("MTAw") == 100 # base64 decoding of "100" + + # Test with None should return 0 + assert page_id_to_offset(None) == 0 + + +def test_bidirectional_conversion(): + # Test converting offset to page_id and back + test_offsets = [0, 1, 10, 100, 1000] + for offset in test_offsets: + page_id = offset_to_page_id(offset, True) + assert page_id_to_offset(page_id) == offset \ No newline at end of file From bdbce31c9c3bc07bd4f6ad1120445e35d506e586 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Fri, 27 Dec 2024 09:46:21 -0700 Subject: [PATCH 09/56] Made tests more generic --- tests/unit/test_search_utils.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/tests/unit/test_search_utils.py b/tests/unit/test_search_utils.py index 7216a9b75e1c..9f7d90b5f314 100644 --- a/tests/unit/test_search_utils.py +++ b/tests/unit/test_search_utils.py @@ -4,9 +4,8 @@ def test_offset_to_page_id(): # Test with has_next=True - assert offset_to_page_id(10, True) == "MTA=" # base64 encoding of "10" - assert offset_to_page_id(0, True) == "MA==" # base64 encoding of "0" - assert offset_to_page_id(100, True) == "MTAw" # base64 encoding of "100" + assert bool(offset_to_page_id(10, True)) + assert bool(offset_to_page_id(0, True)) # Test with has_next=False should return None assert offset_to_page_id(10, False) is None @@ -14,11 +13,6 @@ def test_offset_to_page_id(): def test_page_id_to_offset(): - # Test with valid page_ids - assert page_id_to_offset("MTA=") == 10 # base64 decoding of "10" - assert page_id_to_offset("MA==") == 0 # base64 decoding of "0" - assert page_id_to_offset("MTAw") == 100 # base64 decoding of "100" - # Test with None should return 0 assert page_id_to_offset(None) == 0 From b182ba001d6cf67d9d3150807251ee1a86946901 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Fri, 27 Dec 2024 09:53:11 -0700 Subject: [PATCH 10/56] Removed something I don't know how it got in there --- frontend/public/mockServiceWorker.js | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/frontend/public/mockServiceWorker.js b/frontend/public/mockServiceWorker.js index ec47a9a50a24..fead0b3ff91c 100644 --- a/frontend/public/mockServiceWorker.js +++ b/frontend/public/mockServiceWorker.js @@ -8,8 +8,8 @@ * - Please do NOT serve this file on production. */ -const PACKAGE_VERSION = '2.7.0' -const INTEGRITY_CHECKSUM = '00729d72e3b82faf54ca8b9621dbb96f' +const PACKAGE_VERSION = '2.6.6' +const INTEGRITY_CHECKSUM = 'ca7800994cc8bfb5eb961e037c877074' const IS_MOCKED_RESPONSE = Symbol('isMockedResponse') const activeClientIds = new Set() @@ -199,19 +199,7 @@ async function getResponse(event, client, requestId) { // Remove the "accept" header value that marked this request as passthrough. // This prevents request alteration and also keeps it compliant with the // user-defined CORS policies. - const acceptHeader = headers.get('accept') - if (acceptHeader) { - const values = acceptHeader.split(',').map((value) => value.trim()) - const filteredValues = values.filter( - (value) => value !== 'msw/passthrough', - ) - - if (filteredValues.length > 0) { - headers.set('accept', filteredValues.join(', ')) - } else { - headers.delete('accept') - } - } + headers.delete('accept', 'msw/passthrough') return fetch(requestClone, { headers }) } From ae9ab6db39bec13a3119890872e3b40b9a25ff48 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Fri, 27 Dec 2024 10:31:51 -0700 Subject: [PATCH 11/56] Now returning selected repository --- openhands/server/data_models/conversation_info.py | 2 ++ openhands/server/routes/new_conversation.py | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/openhands/server/data_models/conversation_info.py b/openhands/server/data_models/conversation_info.py index 7e6900c8ae73..c2644d7a2237 100644 --- a/openhands/server/data_models/conversation_info.py +++ b/openhands/server/data_models/conversation_info.py @@ -6,7 +6,9 @@ @dataclass class ConversationInfo: + """ Information about a conversation """ id: str title: str | None = None last_updated_at: datetime | None = None status: ConversationStatus = ConversationStatus.STOPPED + selected_repository: str | None = None diff --git a/openhands/server/routes/new_conversation.py b/openhands/server/routes/new_conversation.py index c0d64f641183..b18604b45027 100644 --- a/openhands/server/routes/new_conversation.py +++ b/openhands/server/routes/new_conversation.py @@ -123,17 +123,17 @@ def _get_conversation_info(session_id: str, is_running: bool, file_store: FileSt metadata = json.loads( file_store.read(f'sessions/{session_id}/metadata.json') ) - title = metadata.get('title', '') events = file_store.list(f'sessions/{session_id}/events/') events = sorted(events) event_path = events[-1] event = json.loads(file_store.read(event_path)) return ConversationInfo( id=session_id, - title=title, + title=metadata.get('title', ''), last_updated_at=datetime.fromisoformat( event.get('timestamp') ), + selected_repository=metadata.get('selected_repository'), status=ConversationStatus.RUNNING if is_running else ConversationStatus.STOPPED, From 74a20683453de4488e9f565bf2ccaeca5a65e7a6 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Fri, 27 Dec 2024 11:00:28 -0700 Subject: [PATCH 12/56] Added get conversation endpoint --- openhands/server/middleware.py | 2 +- openhands/server/routes/new_conversation.py | 70 ++++++++++++--------- 2 files changed, 40 insertions(+), 32 deletions(-) diff --git a/openhands/server/middleware.py b/openhands/server/middleware.py index 46fac1ead29c..a7b30e479bbc 100644 --- a/openhands/server/middleware.py +++ b/openhands/server/middleware.py @@ -121,7 +121,7 @@ def _should_attach(self, request) -> bool: if request.url.path.startswith('/api/conversation'): # FIXME: we should be able to use path_params path_parts = request.url.path.split('/') - if len(path_parts) > 3: + if len(path_parts) > 4: conversation_id = request.url.path.split('/')[3] if not conversation_id: return False diff --git a/openhands/server/routes/new_conversation.py b/openhands/server/routes/new_conversation.py index b18604b45027..1371e7053995 100644 --- a/openhands/server/routes/new_conversation.py +++ b/openhands/server/routes/new_conversation.py @@ -104,37 +104,45 @@ async def search_conversations( set(session_ids[start:end]) ) for session_id in session_ids: - try: - is_running = session_id in running_sessions - conversation_info = _get_conversation_info(session_id, is_running, file_store) - if conversation_info: - conversations.append(conversation_info) - except Exception: # type: ignore - # If a conversation is corrupt, we simply log and skip. - logger.warning( - f'Error loading session: {session_id}', - exc_info=True, - stack_info=True, - ) + is_running = session_id in running_sessions + conversation_info = _get_conversation_info(session_id, is_running, file_store) + if conversation_info: + conversations.append(conversation_info) return ConversationResultSet(results=conversations, next_page_id=next_page_id) -def _get_conversation_info(session_id: str, is_running: bool, file_store: FileStore) -> ConversationInfo: - metadata = json.loads( - file_store.read(f'sessions/{session_id}/metadata.json') - ) - events = file_store.list(f'sessions/{session_id}/events/') - events = sorted(events) - event_path = events[-1] - event = json.loads(file_store.read(event_path)) - return ConversationInfo( - id=session_id, - title=metadata.get('title', ''), - last_updated_at=datetime.fromisoformat( - event.get('timestamp') - ), - selected_repository=metadata.get('selected_repository'), - status=ConversationStatus.RUNNING - if is_running - else ConversationStatus.STOPPED, - ) +def _get_conversation_info(session_id: str, is_running: bool, file_store: FileStore) -> ConversationInfo | None: + try: + metadata = json.loads( + file_store.read(f'sessions/{session_id}/metadata.json') + ) + events = file_store.list(f'sessions/{session_id}/events/') + events = sorted(events) + event_path = events[-1] + event = json.loads(file_store.read(event_path)) + return ConversationInfo( + id=session_id, + title=metadata.get('title', ''), + last_updated_at=datetime.fromisoformat( + event.get('timestamp') + ), + selected_repository=metadata.get('selected_repository'), + status=ConversationStatus.RUNNING + if is_running + else ConversationStatus.STOPPED, + ) + except Exception: # type: ignore + logger.warning( + f'Error loading conversation: {session_id}', + exc_info=True, + stack_info=True, + ) + return None + + +@app.get('/conversation/{conversation_id}') +async def get_conversation(conversation_id: str) -> ConversationInfo | None: + file_store = session_manager.file_store + is_running = await session_manager.is_agent_loop_running(conversation_id) + conversation_info = _get_conversation_info(conversation_id, is_running, file_store) + return conversation_info From 8d0896be6e8ca8ac5b6818fa12d925d5649cf223 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Fri, 27 Dec 2024 11:13:33 -0700 Subject: [PATCH 13/56] Lint fix --- tests/unit/test_search_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_search_utils.py b/tests/unit/test_search_utils.py index 9f7d90b5f314..4318d81e5803 100644 --- a/tests/unit/test_search_utils.py +++ b/tests/unit/test_search_utils.py @@ -22,4 +22,4 @@ def test_bidirectional_conversion(): test_offsets = [0, 1, 10, 100, 1000] for offset in test_offsets: page_id = offset_to_page_id(offset, True) - assert page_id_to_offset(page_id) == offset \ No newline at end of file + assert page_id_to_offset(page_id) == offset From ede28d54a3c5d37718e5f4f4167beda70fcf9fee Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Fri, 27 Dec 2024 11:41:13 -0700 Subject: [PATCH 14/56] dded tests --- openhands/server/routes/new_conversation.py | 1 - tests/unit/test_conversation.py | 59 +++++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 tests/unit/test_conversation.py diff --git a/openhands/server/routes/new_conversation.py b/openhands/server/routes/new_conversation.py index 1371e7053995..c49e8bd61069 100644 --- a/openhands/server/routes/new_conversation.py +++ b/openhands/server/routes/new_conversation.py @@ -1,4 +1,3 @@ -import base64 import json import uuid from datetime import datetime diff --git a/tests/unit/test_conversation.py b/tests/unit/test_conversation.py new file mode 100644 index 000000000000..37c191439b59 --- /dev/null +++ b/tests/unit/test_conversation.py @@ -0,0 +1,59 @@ + + + +from datetime import datetime +import json +from unittest.mock import patch + +import pytest +from openhands.server.data_models.conversation_info import ConversationInfo +from openhands.server.data_models.conversation_result_set import ConversationResultSet +from openhands.server.data_models.conversation_status import ConversationStatus +from openhands.server.routes.new_conversation import get_conversation, search_conversations +from openhands.storage.memory import InMemoryFileStore + + + +@pytest.mark.asyncio +async def test_search_conversations(): + file_store = InMemoryFileStore() + file_store.write('sessions/some_conversation_id/metadata.json', json.dumps({ 'title': 'Some Conversation', 'selected_repository': 'foobar' })) + file_store.write('sessions/some_conversation_id/events/0.json', json.dumps({ 'timestamp': '2025-01-01T00:00:00' })) + with patch('openhands.server.routes.new_conversation.session_manager.file_store', file_store): + result_set = await search_conversations() + expected = ConversationResultSet( + results=[ + ConversationInfo( + id='some_conversation_id', + title='Some Conversation', + last_updated_at=datetime.fromisoformat('2025-01-01T00:00:00'), + status=ConversationStatus.STOPPED, + selected_repository='foobar', + ) + ] + ) + assert result_set == expected + + + +@pytest.mark.asyncio +async def test_get_conversation(): + file_store = InMemoryFileStore() + file_store.write('sessions/some_conversation_id/metadata.json', json.dumps({ 'title': 'Some Conversation', 'selected_repository': 'foobar' })) + file_store.write('sessions/some_conversation_id/events/0.json', json.dumps({ 'timestamp': '2025-01-01T00:00:00' })) + with patch('openhands.server.routes.new_conversation.session_manager.file_store', file_store): + conversation = await get_conversation('some_conversation_id') + expected = ConversationInfo( + id='some_conversation_id', + title='Some Conversation', + last_updated_at=datetime.fromisoformat('2025-01-01T00:00:00'), + status=ConversationStatus.STOPPED, + selected_repository='foobar', + ) + assert conversation == expected + + +@pytest.mark.asyncio +async def test_get_missing_conversation(): + with patch('openhands.server.routes.new_conversation.session_manager.file_store', InMemoryFileStore({})): + assert await get_conversation('no_such_conversation') is None From 6b30f2ae67bd0ed1e1a9609a7331960408b1a669 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Fri, 27 Dec 2024 11:41:53 -0700 Subject: [PATCH 15/56] Lint fixes --- openhands/utils/search_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openhands/utils/search_utils.py b/openhands/utils/search_utils.py index 5b3fd6e9c6e9..315d0775c0c4 100644 --- a/openhands/utils/search_utils.py +++ b/openhands/utils/search_utils.py @@ -12,4 +12,4 @@ def page_id_to_offset(page_id: str | None) -> int: if not page_id: return 0 offset = int(base64.b64decode(page_id).decode()) - return offset \ No newline at end of file + return offset From 7bd01d2ac1c130b6594f5228fb05ba8c29c7ebaa Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Fri, 27 Dec 2024 11:44:35 -0700 Subject: [PATCH 16/56] WIP --- tests/unit/test_search_utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/test_search_utils.py b/tests/unit/test_search_utils.py index 4318d81e5803..3e68dbfab79f 100644 --- a/tests/unit/test_search_utils.py +++ b/tests/unit/test_search_utils.py @@ -1,4 +1,3 @@ -import pytest from openhands.utils.search_utils import offset_to_page_id, page_id_to_offset From 0220b6935250361c21a989232caaf8525d5b1308 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Fri, 27 Dec 2024 11:46:48 -0700 Subject: [PATCH 17/56] Ruff --- .../server/data_models/conversation_info.py | 3 +- openhands/server/routes/new_conversation.py | 12 +++-- tests/unit/test_conversation.py | 45 +++++++++++++------ 3 files changed, 39 insertions(+), 21 deletions(-) diff --git a/openhands/server/data_models/conversation_info.py b/openhands/server/data_models/conversation_info.py index c2644d7a2237..1d3ab6931d5b 100644 --- a/openhands/server/data_models/conversation_info.py +++ b/openhands/server/data_models/conversation_info.py @@ -6,7 +6,8 @@ @dataclass class ConversationInfo: - """ Information about a conversation """ + """Information about a conversation""" + id: str title: str | None = None last_updated_at: datetime | None = None diff --git a/openhands/server/routes/new_conversation.py b/openhands/server/routes/new_conversation.py index c49e8bd61069..a547c3aff4dc 100644 --- a/openhands/server/routes/new_conversation.py +++ b/openhands/server/routes/new_conversation.py @@ -110,11 +110,11 @@ async def search_conversations( return ConversationResultSet(results=conversations, next_page_id=next_page_id) -def _get_conversation_info(session_id: str, is_running: bool, file_store: FileStore) -> ConversationInfo | None: +def _get_conversation_info( + session_id: str, is_running: bool, file_store: FileStore +) -> ConversationInfo | None: try: - metadata = json.loads( - file_store.read(f'sessions/{session_id}/metadata.json') - ) + metadata = json.loads(file_store.read(f'sessions/{session_id}/metadata.json')) events = file_store.list(f'sessions/{session_id}/events/') events = sorted(events) event_path = events[-1] @@ -122,9 +122,7 @@ def _get_conversation_info(session_id: str, is_running: bool, file_store: FileSt return ConversationInfo( id=session_id, title=metadata.get('title', ''), - last_updated_at=datetime.fromisoformat( - event.get('timestamp') - ), + last_updated_at=datetime.fromisoformat(event.get('timestamp')), selected_repository=metadata.get('selected_repository'), status=ConversationStatus.RUNNING if is_running diff --git a/tests/unit/test_conversation.py b/tests/unit/test_conversation.py index 37c191439b59..0c34d5144c74 100644 --- a/tests/unit/test_conversation.py +++ b/tests/unit/test_conversation.py @@ -1,6 +1,3 @@ - - - from datetime import datetime import json from unittest.mock import patch @@ -9,17 +6,28 @@ from openhands.server.data_models.conversation_info import ConversationInfo from openhands.server.data_models.conversation_result_set import ConversationResultSet from openhands.server.data_models.conversation_status import ConversationStatus -from openhands.server.routes.new_conversation import get_conversation, search_conversations +from openhands.server.routes.new_conversation import ( + get_conversation, + search_conversations, +) from openhands.storage.memory import InMemoryFileStore - @pytest.mark.asyncio async def test_search_conversations(): file_store = InMemoryFileStore() - file_store.write('sessions/some_conversation_id/metadata.json', json.dumps({ 'title': 'Some Conversation', 'selected_repository': 'foobar' })) - file_store.write('sessions/some_conversation_id/events/0.json', json.dumps({ 'timestamp': '2025-01-01T00:00:00' })) - with patch('openhands.server.routes.new_conversation.session_manager.file_store', file_store): + file_store.write( + 'sessions/some_conversation_id/metadata.json', + json.dumps({'title': 'Some Conversation', 'selected_repository': 'foobar'}), + ) + file_store.write( + 'sessions/some_conversation_id/events/0.json', + json.dumps({'timestamp': '2025-01-01T00:00:00'}), + ) + with patch( + 'openhands.server.routes.new_conversation.session_manager.file_store', + file_store, + ): result_set = await search_conversations() expected = ConversationResultSet( results=[ @@ -35,13 +43,21 @@ async def test_search_conversations(): assert result_set == expected - @pytest.mark.asyncio async def test_get_conversation(): file_store = InMemoryFileStore() - file_store.write('sessions/some_conversation_id/metadata.json', json.dumps({ 'title': 'Some Conversation', 'selected_repository': 'foobar' })) - file_store.write('sessions/some_conversation_id/events/0.json', json.dumps({ 'timestamp': '2025-01-01T00:00:00' })) - with patch('openhands.server.routes.new_conversation.session_manager.file_store', file_store): + file_store.write( + 'sessions/some_conversation_id/metadata.json', + json.dumps({'title': 'Some Conversation', 'selected_repository': 'foobar'}), + ) + file_store.write( + 'sessions/some_conversation_id/events/0.json', + json.dumps({'timestamp': '2025-01-01T00:00:00'}), + ) + with patch( + 'openhands.server.routes.new_conversation.session_manager.file_store', + file_store, + ): conversation = await get_conversation('some_conversation_id') expected = ConversationInfo( id='some_conversation_id', @@ -55,5 +71,8 @@ async def test_get_conversation(): @pytest.mark.asyncio async def test_get_missing_conversation(): - with patch('openhands.server.routes.new_conversation.session_manager.file_store', InMemoryFileStore({})): + with patch( + 'openhands.server.routes.new_conversation.session_manager.file_store', + InMemoryFileStore({}), + ): assert await get_conversation('no_such_conversation') is None From 41723655b51eb61e9dac34e19fb0dbbbe223a337 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Fri, 27 Dec 2024 11:48:35 -0700 Subject: [PATCH 18/56] Lint fix --- tests/unit/test_conversation.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_conversation.py b/tests/unit/test_conversation.py index 0c34d5144c74..6cdf38d57dbb 100644 --- a/tests/unit/test_conversation.py +++ b/tests/unit/test_conversation.py @@ -1,8 +1,9 @@ -from datetime import datetime import json +from datetime import datetime from unittest.mock import patch import pytest + from openhands.server.data_models.conversation_info import ConversationInfo from openhands.server.data_models.conversation_result_set import ConversationResultSet from openhands.server.data_models.conversation_status import ConversationStatus From 4305335e59a7e83fc92d8ae3d8bb1d38b960af85 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Fri, 27 Dec 2024 13:42:31 -0700 Subject: [PATCH 19/56] WIP --- openhands/server/routes/new_conversation.py | 48 ++++++++++++++----- .../conversation/conversation_store.py | 7 ++- 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/openhands/server/routes/new_conversation.py b/openhands/server/routes/new_conversation.py index a547c3aff4dc..3e21a69c075a 100644 --- a/openhands/server/routes/new_conversation.py +++ b/openhands/server/routes/new_conversation.py @@ -19,6 +19,7 @@ ConversationStore, ) from openhands.storage.files import FileStore +from openhands.storage.locations import get_conversation_events_dir from openhands.utils.async_utils import call_sync_from_async from openhands.utils.search_utils import offset_to_page_id, page_id_to_offset @@ -98,32 +99,33 @@ async def search_conversations( num_sessions = len(session_ids) start = page_id_to_offset(page_id) end = min(limit + start, num_sessions) + session_ids = session_ids[start:end] next_page_id = offset_to_page_id(end, end < num_sessions) - running_sessions = await session_manager.get_agent_loop_running( - set(session_ids[start:end]) - ) + running_sessions = await session_manager.get_agent_loop_running(set(session_ids)) for session_id in session_ids: is_running = session_id in running_sessions - conversation_info = _get_conversation_info(session_id, is_running, file_store) + conversation_info = await _get_conversation_info(session_id, is_running, file_store) if conversation_info: conversations.append(conversation_info) return ConversationResultSet(results=conversations, next_page_id=next_page_id) -def _get_conversation_info( +async def _get_conversation_info( session_id: str, is_running: bool, file_store: FileStore ) -> ConversationInfo | None: try: - metadata = json.loads(file_store.read(f'sessions/{session_id}/metadata.json')) - events = file_store.list(f'sessions/{session_id}/events/') + conversation_store = await ConversationStore.get_instance(config) + metadata = await conversation_store.get_metadata(session_id) + events_dir = get_conversation_events_dir(session_id) + events = file_store.list(events_dir) events = sorted(events) event_path = events[-1] event = json.loads(file_store.read(event_path)) return ConversationInfo( id=session_id, - title=metadata.get('title', ''), + title=metadata.title, last_updated_at=datetime.fromisoformat(event.get('timestamp')), - selected_repository=metadata.get('selected_repository'), + selected_repository=metadata.selected_repository, status=ConversationStatus.RUNNING if is_running else ConversationStatus.STOPPED, @@ -137,9 +139,33 @@ def _get_conversation_info( return None -@app.get('/conversation/{conversation_id}') +@app.get('/conversations/{conversation_id}') async def get_conversation(conversation_id: str) -> ConversationInfo | None: file_store = session_manager.file_store is_running = await session_manager.is_agent_loop_running(conversation_id) - conversation_info = _get_conversation_info(conversation_id, is_running, file_store) + conversation_info = await _get_conversation_info(conversation_id, is_running, file_store) return conversation_info + + +@app.post('/conversations/{conversation_id}') +async def update_conversation(conversation_id: str, title: str) -> bool: + conversation_store = await ConversationStore.get_instance(config) + metadata = await conversation_store.get_metadata(conversation_id) + if not metadata: + return False + metadata.title = title + await conversation_store.save_metadata(metadata) + return True + + +@app.delete('/conversations/{conversation_id}') +async def delete_conversation(conversation_id: str) -> bool: + conversation_store = await ConversationStore.get_instance(config) + metadata = await conversation_store.get_metadata(conversation_id) + if not metadata or metadata: + return False + is_running = await session_manager.is_agent_loop_running(conversation_id) + if is_running: + return False + await conversation_store.delete_metadata(conversation_id) + return True diff --git a/openhands/storage/conversation/conversation_store.py b/openhands/storage/conversation/conversation_store.py index 461c1a98b707..cc5a7f5e986e 100644 --- a/openhands/storage/conversation/conversation_store.py +++ b/openhands/storage/conversation/conversation_store.py @@ -4,7 +4,7 @@ from openhands.core.config.app_config import AppConfig from openhands.storage import get_file_store from openhands.storage.files import FileStore -from openhands.storage.locations import get_conversation_metadata_filename +from openhands.storage.locations import get_conversation_dir, get_conversation_metadata_filename from openhands.utils.async_utils import call_sync_from_async @@ -13,6 +13,7 @@ class ConversationMetadata: conversation_id: str github_user_id: str selected_repository: str | None + title: str | None = None @dataclass @@ -28,6 +29,10 @@ async def get_metadata(self, conversation_id: str) -> ConversationMetadata: path = get_conversation_metadata_filename(conversation_id) json_str = await call_sync_from_async(self.file_store.read, path) return ConversationMetadata(**json.loads(json_str)) + + async def delete_metadata(self, conversation_id: str) -> None: + conversation_dir = get_conversation_dir(conversation_id) + await call_sync_from_async(self.file_store.delete, conversation_dir) async def exists(self, conversation_id: str) -> bool: path = get_conversation_metadata_filename(conversation_id) From a90dd21631ec9eaf4a79795c41006fcdda0e4f0b Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Fri, 27 Dec 2024 14:13:43 -0700 Subject: [PATCH 20/56] Fix tests --- tests/unit/test_conversation.py | 76 ++++++++++++++++++++------------- 1 file changed, 47 insertions(+), 29 deletions(-) diff --git a/tests/unit/test_conversation.py b/tests/unit/test_conversation.py index 6cdf38d57dbb..cf8cbec8ef0c 100644 --- a/tests/unit/test_conversation.py +++ b/tests/unit/test_conversation.py @@ -1,6 +1,6 @@ import json from datetime import datetime -from unittest.mock import patch +from unittest.mock import MagicMock, patch import pytest @@ -19,29 +19,38 @@ async def test_search_conversations(): file_store = InMemoryFileStore() file_store.write( 'sessions/some_conversation_id/metadata.json', - json.dumps({'title': 'Some Conversation', 'selected_repository': 'foobar'}), + json.dumps({ + 'title': 'Some Conversation', + 'selected_repository': 'foobar', + 'conversation_id': 'some_conversation_id', + 'github_user_id': 'github_user', + }), ) file_store.write( 'sessions/some_conversation_id/events/0.json', json.dumps({'timestamp': '2025-01-01T00:00:00'}), ) with patch( - 'openhands.server.routes.new_conversation.session_manager.file_store', - file_store, + 'openhands.storage.conversation.conversation_store.get_file_store', + MagicMock(return_value=file_store) ): - result_set = await search_conversations() - expected = ConversationResultSet( - results=[ - ConversationInfo( - id='some_conversation_id', - title='Some Conversation', - last_updated_at=datetime.fromisoformat('2025-01-01T00:00:00'), - status=ConversationStatus.STOPPED, - selected_repository='foobar', - ) - ] - ) - assert result_set == expected + with patch( + 'openhands.server.routes.new_conversation.session_manager.file_store', + file_store, + ): + result_set = await search_conversations() + expected = ConversationResultSet( + results=[ + ConversationInfo( + id='some_conversation_id', + title='Some Conversation', + last_updated_at=datetime.fromisoformat('2025-01-01T00:00:00'), + status=ConversationStatus.STOPPED, + selected_repository='foobar', + ) + ] + ) + assert result_set == expected @pytest.mark.asyncio @@ -49,25 +58,34 @@ async def test_get_conversation(): file_store = InMemoryFileStore() file_store.write( 'sessions/some_conversation_id/metadata.json', - json.dumps({'title': 'Some Conversation', 'selected_repository': 'foobar'}), + json.dumps({ + 'title': 'Some Conversation', + 'selected_repository': 'foobar', + 'conversation_id': 'some_conversation_id', + 'github_user_id': 'github_user', + }), ) file_store.write( 'sessions/some_conversation_id/events/0.json', json.dumps({'timestamp': '2025-01-01T00:00:00'}), ) with patch( - 'openhands.server.routes.new_conversation.session_manager.file_store', - file_store, + 'openhands.storage.conversation.conversation_store.get_file_store', + MagicMock(return_value=file_store) ): - conversation = await get_conversation('some_conversation_id') - expected = ConversationInfo( - id='some_conversation_id', - title='Some Conversation', - last_updated_at=datetime.fromisoformat('2025-01-01T00:00:00'), - status=ConversationStatus.STOPPED, - selected_repository='foobar', - ) - assert conversation == expected + with patch( + 'openhands.server.routes.new_conversation.session_manager.file_store', + file_store, + ): + conversation = await get_conversation('some_conversation_id') + expected = ConversationInfo( + id='some_conversation_id', + title='Some Conversation', + last_updated_at=datetime.fromisoformat('2025-01-01T00:00:00'), + status=ConversationStatus.STOPPED, + selected_repository='foobar', + ) + assert conversation == expected @pytest.mark.asyncio From a8379d7f305f8594b181ba44a357d2643f92dcc3 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Fri, 27 Dec 2024 14:15:16 -0700 Subject: [PATCH 21/56] WIP --- tests/unit/test_conversation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_conversation.py b/tests/unit/test_conversation.py index cf8cbec8ef0c..a3b4919e91c9 100644 --- a/tests/unit/test_conversation.py +++ b/tests/unit/test_conversation.py @@ -32,7 +32,7 @@ async def test_search_conversations(): ) with patch( 'openhands.storage.conversation.conversation_store.get_file_store', - MagicMock(return_value=file_store) + MagicMock(return_value=file_store), ): with patch( 'openhands.server.routes.new_conversation.session_manager.file_store', @@ -71,7 +71,7 @@ async def test_get_conversation(): ) with patch( 'openhands.storage.conversation.conversation_store.get_file_store', - MagicMock(return_value=file_store) + MagicMock(return_value=file_store), ): with patch( 'openhands.server.routes.new_conversation.session_manager.file_store', From e9d3f4b07c0bc627321c67e6036e6dfedb7a79ab Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Fri, 27 Dec 2024 14:25:25 -0700 Subject: [PATCH 22/56] WIP --- openhands/server/routes/new_conversation.py | 8 +- .../conversation/conversation_store.py | 7 +- tests/unit/test_conversation.py | 92 ++++++++----------- 3 files changed, 51 insertions(+), 56 deletions(-) diff --git a/openhands/server/routes/new_conversation.py b/openhands/server/routes/new_conversation.py index 3e21a69c075a..eab4c395d506 100644 --- a/openhands/server/routes/new_conversation.py +++ b/openhands/server/routes/new_conversation.py @@ -104,7 +104,9 @@ async def search_conversations( running_sessions = await session_manager.get_agent_loop_running(set(session_ids)) for session_id in session_ids: is_running = session_id in running_sessions - conversation_info = await _get_conversation_info(session_id, is_running, file_store) + conversation_info = await _get_conversation_info( + session_id, is_running, file_store + ) if conversation_info: conversations.append(conversation_info) return ConversationResultSet(results=conversations, next_page_id=next_page_id) @@ -143,7 +145,9 @@ async def _get_conversation_info( async def get_conversation(conversation_id: str) -> ConversationInfo | None: file_store = session_manager.file_store is_running = await session_manager.is_agent_loop_running(conversation_id) - conversation_info = await _get_conversation_info(conversation_id, is_running, file_store) + conversation_info = await _get_conversation_info( + conversation_id, is_running, file_store + ) return conversation_info diff --git a/openhands/storage/conversation/conversation_store.py b/openhands/storage/conversation/conversation_store.py index cc5a7f5e986e..d73d6b680ac9 100644 --- a/openhands/storage/conversation/conversation_store.py +++ b/openhands/storage/conversation/conversation_store.py @@ -4,7 +4,10 @@ from openhands.core.config.app_config import AppConfig from openhands.storage import get_file_store from openhands.storage.files import FileStore -from openhands.storage.locations import get_conversation_dir, get_conversation_metadata_filename +from openhands.storage.locations import ( + get_conversation_dir, + get_conversation_metadata_filename, +) from openhands.utils.async_utils import call_sync_from_async @@ -29,7 +32,7 @@ async def get_metadata(self, conversation_id: str) -> ConversationMetadata: path = get_conversation_metadata_filename(conversation_id) json_str = await call_sync_from_async(self.file_store.read, path) return ConversationMetadata(**json.loads(json_str)) - + async def delete_metadata(self, conversation_id: str) -> None: conversation_dir = get_conversation_dir(conversation_id) await call_sync_from_async(self.file_store.delete, conversation_dir) diff --git a/tests/unit/test_conversation.py b/tests/unit/test_conversation.py index a3b4919e91c9..c983815625b3 100644 --- a/tests/unit/test_conversation.py +++ b/tests/unit/test_conversation.py @@ -1,3 +1,4 @@ +from contextlib import contextmanager import json from datetime import datetime from unittest.mock import MagicMock, patch @@ -14,17 +15,19 @@ from openhands.storage.memory import InMemoryFileStore -@pytest.mark.asyncio -async def test_search_conversations(): +@contextmanager +def _patch_store(): file_store = InMemoryFileStore() file_store.write( 'sessions/some_conversation_id/metadata.json', - json.dumps({ - 'title': 'Some Conversation', - 'selected_repository': 'foobar', - 'conversation_id': 'some_conversation_id', - 'github_user_id': 'github_user', - }), + json.dumps( + { + 'title': 'Some Conversation', + 'selected_repository': 'foobar', + 'conversation_id': 'some_conversation_id', + 'github_user_id': 'github_user', + } + ), ) file_store.write( 'sessions/some_conversation_id/events/0.json', @@ -38,54 +41,39 @@ async def test_search_conversations(): 'openhands.server.routes.new_conversation.session_manager.file_store', file_store, ): - result_set = await search_conversations() - expected = ConversationResultSet( - results=[ - ConversationInfo( - id='some_conversation_id', - title='Some Conversation', - last_updated_at=datetime.fromisoformat('2025-01-01T00:00:00'), - status=ConversationStatus.STOPPED, - selected_repository='foobar', - ) - ] - ) - assert result_set == expected + yield + + +@pytest.mark.asyncio +async def test_search_conversations(): + with _patch_store(): + result_set = await search_conversations() + expected = ConversationResultSet( + results=[ + ConversationInfo( + id='some_conversation_id', + title='Some Conversation', + last_updated_at=datetime.fromisoformat('2025-01-01T00:00:00'), + status=ConversationStatus.STOPPED, + selected_repository='foobar', + ) + ] + ) + assert result_set == expected @pytest.mark.asyncio async def test_get_conversation(): - file_store = InMemoryFileStore() - file_store.write( - 'sessions/some_conversation_id/metadata.json', - json.dumps({ - 'title': 'Some Conversation', - 'selected_repository': 'foobar', - 'conversation_id': 'some_conversation_id', - 'github_user_id': 'github_user', - }), - ) - file_store.write( - 'sessions/some_conversation_id/events/0.json', - json.dumps({'timestamp': '2025-01-01T00:00:00'}), - ) - with patch( - 'openhands.storage.conversation.conversation_store.get_file_store', - MagicMock(return_value=file_store), - ): - with patch( - 'openhands.server.routes.new_conversation.session_manager.file_store', - file_store, - ): - conversation = await get_conversation('some_conversation_id') - expected = ConversationInfo( - id='some_conversation_id', - title='Some Conversation', - last_updated_at=datetime.fromisoformat('2025-01-01T00:00:00'), - status=ConversationStatus.STOPPED, - selected_repository='foobar', - ) - assert conversation == expected + with _patch_store(): + conversation = await get_conversation('some_conversation_id') + expected = ConversationInfo( + id='some_conversation_id', + title='Some Conversation', + last_updated_at=datetime.fromisoformat('2025-01-01T00:00:00'), + status=ConversationStatus.STOPPED, + selected_repository='foobar', + ) + assert conversation == expected @pytest.mark.asyncio From 02cd435a0a023ec01016e56f3697695e9c9b2e6c Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Fri, 27 Dec 2024 14:28:46 -0700 Subject: [PATCH 23/56] WIP --- tests/unit/test_conversation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_conversation.py b/tests/unit/test_conversation.py index c983815625b3..73100a8a6d57 100644 --- a/tests/unit/test_conversation.py +++ b/tests/unit/test_conversation.py @@ -1,5 +1,5 @@ -from contextlib import contextmanager import json +from contextlib import contextmanager from datetime import datetime from unittest.mock import MagicMock, patch From d7ccd78c1be71ba31136542b92640554371526c4 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Fri, 27 Dec 2024 14:33:59 -0700 Subject: [PATCH 24/56] WIP --- openhands/server/routes/new_conversation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openhands/server/routes/new_conversation.py b/openhands/server/routes/new_conversation.py index eab4c395d506..743aeb980a8f 100644 --- a/openhands/server/routes/new_conversation.py +++ b/openhands/server/routes/new_conversation.py @@ -166,7 +166,7 @@ async def update_conversation(conversation_id: str, title: str) -> bool: async def delete_conversation(conversation_id: str) -> bool: conversation_store = await ConversationStore.get_instance(config) metadata = await conversation_store.get_metadata(conversation_id) - if not metadata or metadata: + if not metadata: return False is_running = await session_manager.is_agent_loop_running(conversation_id) if is_running: From 64d8dc8b70a71dacd4b95d018efd186d335303dc Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Fri, 27 Dec 2024 14:37:11 -0700 Subject: [PATCH 25/56] WIP --- openhands/server/routes/new_conversation.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/openhands/server/routes/new_conversation.py b/openhands/server/routes/new_conversation.py index 743aeb980a8f..ef9196dc68f2 100644 --- a/openhands/server/routes/new_conversation.py +++ b/openhands/server/routes/new_conversation.py @@ -165,8 +165,9 @@ async def update_conversation(conversation_id: str, title: str) -> bool: @app.delete('/conversations/{conversation_id}') async def delete_conversation(conversation_id: str) -> bool: conversation_store = await ConversationStore.get_instance(config) - metadata = await conversation_store.get_metadata(conversation_id) - if not metadata: + try: + await conversation_store.get_metadata(conversation_id) + except FileNotFoundError: return False is_running = await session_manager.is_agent_loop_running(conversation_id) if is_running: From 858d4cbec056aed2bc5f352935cde0c642e323a4 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Mon, 30 Dec 2024 16:14:17 -0700 Subject: [PATCH 26/56] Ruff --- openhands/server/routes/settings.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/openhands/server/routes/settings.py b/openhands/server/routes/settings.py index 9cb7d0105c26..f6d37d1e674c 100644 --- a/openhands/server/routes/settings.py +++ b/openhands/server/routes/settings.py @@ -12,7 +12,8 @@ SettingsStoreImpl = get_impl(SettingsStore, openhands_config.settings_store_class) # type: ignore ConversationStoreImpl = get_impl( - ConversationStore, openhands_config.conversation_store_class # type: ignore + ConversationStore, + openhands_config.conversation_store_class, # type: ignore ) From 4181d0916dd9cca1620e30a65f3e7fe6557b9c66 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Tue, 31 Dec 2024 08:33:03 -0700 Subject: [PATCH 27/56] WIP --- .../data_models/conversation_metadata.py | 1 + openhands/server/routes/new_conversation.py | 20 +++++++++---------- openhands/server/routes/settings.py | 8 ++------ 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/openhands/server/data_models/conversation_metadata.py b/openhands/server/data_models/conversation_metadata.py index 8aa43a623bd9..fbe6ef2535e8 100644 --- a/openhands/server/data_models/conversation_metadata.py +++ b/openhands/server/data_models/conversation_metadata.py @@ -6,3 +6,4 @@ class ConversationMetadata: conversation_id: str github_user_id: str selected_repository: str | None + title: str | None = None diff --git a/openhands/server/routes/new_conversation.py b/openhands/server/routes/new_conversation.py index 3399e03c8bce..7412ddf39254 100644 --- a/openhands/server/routes/new_conversation.py +++ b/openhands/server/routes/new_conversation.py @@ -36,10 +36,7 @@ async def new_conversation(request: Request, data: InitSessionRequest): After successful initialization, the client should connect to the WebSocket using the returned conversation ID """ - github_token = '' - if data.github_token: - github_token = data.github_token - + github_token = data.github_token or '' settings_store = await SettingsStoreImpl.get_instance(config, github_token) settings = await settings_store.load() @@ -110,10 +107,11 @@ async def search_conversations( async def _get_conversation_info( - session_id: str, is_running: bool, file_store: FileStore + session_id: str, is_running: bool, file_store: FileStore, request: Request, ) -> ConversationInfo | None: try: - conversation_store = await ConversationStoreImpl.get_instance(config) + github_token = getattr(request.state, 'github_token', '') or '' + conversation_store = await ConversationStoreImpl.get_instance(config, github_token) metadata = await conversation_store.get_metadata(session_id) events_dir = get_conversation_events_dir(session_id) events = file_store.list(events_dir) @@ -149,8 +147,9 @@ async def get_conversation(conversation_id: str) -> ConversationInfo | None: @app.post('/conversations/{conversation_id}') -async def update_conversation(conversation_id: str, title: str) -> bool: - conversation_store = await ConversationStoreImpl.get_instance(config) +async def update_conversation(conversation_id: str, title: str, request: Request) -> bool: + github_token = getattr(request.state, 'github_token', '') or '' + conversation_store = await ConversationStoreImpl.get_instance(config, github_token) metadata = await conversation_store.get_metadata(conversation_id) if not metadata: return False @@ -160,8 +159,9 @@ async def update_conversation(conversation_id: str, title: str) -> bool: @app.delete('/conversations/{conversation_id}') -async def delete_conversation(conversation_id: str) -> bool: - conversation_store = await ConversationStoreImpl.get_instance(config) +async def delete_conversation(conversation_id: str, request: Request,) -> bool: + github_token = getattr(request.state, 'github_token', '') or '' + conversation_store = await ConversationStoreImpl.get_instance(config, github_token) try: await conversation_store.get_metadata(conversation_id) except FileNotFoundError: diff --git a/openhands/server/routes/settings.py b/openhands/server/routes/settings.py index 456bb2a87377..83e150b551cc 100644 --- a/openhands/server/routes/settings.py +++ b/openhands/server/routes/settings.py @@ -18,12 +18,8 @@ @app.get('/settings') -async def load_settings( - request: Request, -) -> Settings | None: - github_token = '' - if hasattr(request.state, 'github_token'): - github_token = request.state.github_token +async def load_settings(request: Request) -> Settings | None: + github_token = getattr(request.state, 'github_token', '') or '' try: settings_store = await SettingsStoreImpl.get_instance(config, github_token) settings = await settings_store.load() From c2041084285309223334749bb40b285ba5de3614 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Tue, 31 Dec 2024 08:33:39 -0700 Subject: [PATCH 28/56] Ruff --- openhands/server/routes/new_conversation.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/openhands/server/routes/new_conversation.py b/openhands/server/routes/new_conversation.py index 7412ddf39254..a0b13a5da6c7 100644 --- a/openhands/server/routes/new_conversation.py +++ b/openhands/server/routes/new_conversation.py @@ -107,11 +107,16 @@ async def search_conversations( async def _get_conversation_info( - session_id: str, is_running: bool, file_store: FileStore, request: Request, + session_id: str, + is_running: bool, + file_store: FileStore, + request: Request, ) -> ConversationInfo | None: try: github_token = getattr(request.state, 'github_token', '') or '' - conversation_store = await ConversationStoreImpl.get_instance(config, github_token) + conversation_store = await ConversationStoreImpl.get_instance( + config, github_token + ) metadata = await conversation_store.get_metadata(session_id) events_dir = get_conversation_events_dir(session_id) events = file_store.list(events_dir) @@ -147,7 +152,9 @@ async def get_conversation(conversation_id: str) -> ConversationInfo | None: @app.post('/conversations/{conversation_id}') -async def update_conversation(conversation_id: str, title: str, request: Request) -> bool: +async def update_conversation( + conversation_id: str, title: str, request: Request +) -> bool: github_token = getattr(request.state, 'github_token', '') or '' conversation_store = await ConversationStoreImpl.get_instance(config, github_token) metadata = await conversation_store.get_metadata(conversation_id) @@ -159,7 +166,10 @@ async def update_conversation(conversation_id: str, title: str, request: Request @app.delete('/conversations/{conversation_id}') -async def delete_conversation(conversation_id: str, request: Request,) -> bool: +async def delete_conversation( + conversation_id: str, + request: Request, +) -> bool: github_token = getattr(request.state, 'github_token', '') or '' conversation_store = await ConversationStoreImpl.get_instance(config, github_token) try: From 9d171b5722fad60768c34baebeba8ea77970113a Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Tue, 31 Dec 2024 12:41:09 -0700 Subject: [PATCH 29/56] WIP --- openhands/server/routes/new_conversation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openhands/server/routes/new_conversation.py b/openhands/server/routes/new_conversation.py index b5bd9ba2d59c..d1a454cbd1f1 100644 --- a/openhands/server/routes/new_conversation.py +++ b/openhands/server/routes/new_conversation.py @@ -77,9 +77,9 @@ async def new_conversation(request: Request, data: InitSessionRequest): @app.get('/conversations') async def search_conversations( + request: Request, page_id: str | None = None, limit: int = 20, - request: Request, ) -> ConversationResultSet: file_store = session_manager.file_store conversations = [] From 370797185f2d6867b5eb15f13baabcb09be2fa07 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Tue, 31 Dec 2024 15:03:22 -0700 Subject: [PATCH 30/56] WIP --- ...set.py => conversation_info_result_set.py} | 2 +- .../conversation_metadata_result_set.py | 9 +++ openhands/server/routes/new_conversation.py | 65 ++++++++----------- .../conversation/conversation_store.py | 9 +++ .../conversation/file_conversation_store.py | 37 ++++++++++- tests/unit/test_conversation.py | 2 +- 6 files changed, 82 insertions(+), 42 deletions(-) rename openhands/server/data_models/{conversation_result_set.py => conversation_info_result_set.py} (87%) create mode 100644 openhands/server/data_models/conversation_metadata_result_set.py diff --git a/openhands/server/data_models/conversation_result_set.py b/openhands/server/data_models/conversation_info_result_set.py similarity index 87% rename from openhands/server/data_models/conversation_result_set.py rename to openhands/server/data_models/conversation_info_result_set.py index d6a203b0062a..5f91f5f531b3 100644 --- a/openhands/server/data_models/conversation_result_set.py +++ b/openhands/server/data_models/conversation_info_result_set.py @@ -4,6 +4,6 @@ @dataclass -class ConversationResultSet: +class ConversationInfoResultSet: results: list[ConversationInfo] = field(default_factory=list) next_page_id: str | None = None diff --git a/openhands/server/data_models/conversation_metadata_result_set.py b/openhands/server/data_models/conversation_metadata_result_set.py new file mode 100644 index 000000000000..74cfc6c8853a --- /dev/null +++ b/openhands/server/data_models/conversation_metadata_result_set.py @@ -0,0 +1,9 @@ +from dataclasses import dataclass, field + +from openhands.server.data_models.conversation_metadata import ConversationMetadata + + +@dataclass +class ConversationMetadataResultSet: + results: list[ConversationMetadata] = field(default_factory=list) + next_page_id: str | None = None diff --git a/openhands/server/routes/new_conversation.py b/openhands/server/routes/new_conversation.py index d1a454cbd1f1..714f61e395fa 100644 --- a/openhands/server/routes/new_conversation.py +++ b/openhands/server/routes/new_conversation.py @@ -10,14 +10,14 @@ from openhands.core.logger import openhands_logger as logger from openhands.server.data_models.conversation_info import ConversationInfo from openhands.server.data_models.conversation_metadata import ConversationMetadata -from openhands.server.data_models.conversation_result_set import ConversationResultSet +from openhands.server.data_models.conversation_info_result_set import ConversationInfoResultSet from openhands.server.data_models.conversation_status import ConversationStatus from openhands.server.routes.settings import ConversationStoreImpl, SettingsStoreImpl from openhands.server.session.conversation_init_data import ConversationInitData from openhands.server.shared import config, session_manager from openhands.storage.files import FileStore from openhands.storage.locations import get_conversation_events_dir -from openhands.utils.async_utils import call_sync_from_async +from openhands.utils.async_utils import call_sync_from_async, wait_all from openhands.utils.search_utils import offset_to_page_id, page_id_to_offset app = APIRouter(prefix='/api') @@ -47,9 +47,7 @@ async def new_conversation(request: Request, data: InitSessionRequest): session_init_args['github_token'] = github_token session_init_args['selected_repository'] = data.selected_repository conversation_init_data = ConversationInitData(**session_init_args) - conversation_store = await ConversationStoreImpl.get_instance(config, github_token) - conversation_id = uuid.uuid4().hex while await conversation_store.exists(conversation_id): logger.warning(f'Collision on conversation ID: {conversation_id}. Retrying...') @@ -80,59 +78,48 @@ async def search_conversations( request: Request, page_id: str | None = None, limit: int = 20, -) -> ConversationResultSet: - file_store = session_manager.file_store - conversations = [] - session_ids = [ - path.split('/')[1] - for path in file_store.list('sessions/') - if not path.startswith('sessions/.') - ] - num_sessions = len(session_ids) - start = page_id_to_offset(page_id) - end = min(limit + start, num_sessions) - session_ids = session_ids[start:end] - next_page_id = offset_to_page_id(end, end < num_sessions) - running_sessions = await session_manager.get_agent_loop_running(set(session_ids)) - for session_id in session_ids: - is_running = session_id in running_sessions - conversation_info = await _get_conversation_info( - session_id, is_running, file_store, request - ) - if conversation_info: - conversations.append(conversation_info) - return ConversationResultSet(results=conversations, next_page_id=next_page_id) +) -> ConversationInfoResultSet: + github_token = getattr(request.state, 'github_token', '') or '' + conversation_store = await ConversationStoreImpl.get_instance(config, github_token) + conversation_metadata_result_set = await conversation_store.search(page_id, limit) + conversation_ids = set(conversation.conversation_id for conversation in conversation_metadata_result_set.results) + running_conversations = await session_manager.get_agent_loop_running(set(conversation_ids)) + result = ConversationInfoResultSet( + results=await wait_all( + _get_conversation_info( + conversation=conversation, + is_running=conversation.conversation_id in running_conversations + ) + for conversation in conversation_metadata_result_set.results + ), + next_page_id=conversation_metadata_result_set.next_page_id + ) + return result async def _get_conversation_info( - session_id: str, + conversation: ConversationMetadata, is_running: bool, - file_store: FileStore, - request: Request, ) -> ConversationInfo | None: try: - github_token = getattr(request.state, 'github_token', '') or '' - conversation_store = await ConversationStoreImpl.get_instance( - config, github_token - ) - metadata = await conversation_store.get_metadata(session_id) - events_dir = get_conversation_events_dir(session_id) + file_store = session_manager.file_store + events_dir = get_conversation_events_dir(conversation.conversation_id) events = file_store.list(events_dir) events = sorted(events) event_path = events[-1] event = json.loads(file_store.read(event_path)) return ConversationInfo( - id=session_id, - title=metadata.title, + id=conversation.conversation_id, + title=conversation.title, last_updated_at=datetime.fromisoformat(event.get('timestamp')), - selected_repository=metadata.selected_repository, + selected_repository=conversation.selected_repository, status=ConversationStatus.RUNNING if is_running else ConversationStatus.STOPPED, ) except Exception: # type: ignore logger.warning( - f'Error loading conversation: {session_id}', + f'Error loading conversation: {conversation.conversation_id}', exc_info=True, stack_info=True, ) diff --git a/openhands/storage/conversation/conversation_store.py b/openhands/storage/conversation/conversation_store.py index c37c72979991..bf479fc1e7a3 100644 --- a/openhands/storage/conversation/conversation_store.py +++ b/openhands/storage/conversation/conversation_store.py @@ -4,6 +4,7 @@ from openhands.core.config.app_config import AppConfig from openhands.server.data_models.conversation_metadata import ConversationMetadata +from openhands.server.data_models.conversation_metadata_result_set import ConversationMetadataResultSet class ConversationStore(ABC): @@ -27,6 +28,14 @@ async def delete_metadata(self, conversation_id: str) -> None: async def exists(self, conversation_id: str) -> bool: """Check if conversation exists""" + @abstractmethod + async def search( + self, + page_id: str | None = None, + limit: int = 20, + ) -> ConversationMetadataResultSet: + """Search conversations""" + @classmethod @abstractmethod async def get_instance( diff --git a/openhands/storage/conversation/file_conversation_store.py b/openhands/storage/conversation/file_conversation_store.py index 0e24119fe81d..f412617fcbc4 100644 --- a/openhands/storage/conversation/file_conversation_store.py +++ b/openhands/storage/conversation/file_conversation_store.py @@ -4,12 +4,15 @@ from dataclasses import dataclass from openhands.core.config.app_config import AppConfig +from openhands.core.logger import openhands_logger as logger +from openhands.server.data_models.conversation_metadata_result_set import ConversationMetadataResultSet from openhands.storage import get_file_store from openhands.storage.conversation.conversation_store import ConversationStore from openhands.server.data_models.conversation_metadata import ConversationMetadata from openhands.storage.files import FileStore -from openhands.storage.locations import get_conversation_metadata_filename +from openhands.storage.locations import CONVERSATION_BASE_DIR, get_conversation_metadata_filename from openhands.utils.async_utils import call_sync_from_async +from openhands.utils.search_utils import offset_to_page_id, page_id_to_offset @dataclass @@ -37,6 +40,38 @@ async def exists(self, conversation_id: str) -> bool: return True except FileNotFoundError: return False + + async def search( + self, + page_id: str | None = None, + limit: int = 20, + ) -> ConversationMetadataResultSet: + conversations = [] + metadata_dir = self.get_conversation_metadata_dir() + conversation_ids = [ + path.split('/')[-2] + for path in self.file_store.list(metadata_dir) + if not path.startswith(f'{metadata_dir}/.') + ] + num_conversations = len(conversation_ids) + start = page_id_to_offset(page_id) + end = min(limit + start, num_conversations) + conversation_ids = conversation_ids[start:end] + conversations = [] + for conversation_id in conversation_ids: + try: + conversations.append(await self.get_metadata(conversation_id)) + except Exception: + logger.warning( + f'Error loading conversation: {conversation_id}', + exc_info=True, + stack_info=True, + ) + next_page_id = offset_to_page_id(end, end < num_conversations) + return ConversationMetadataResultSet(conversations, next_page_id) + + def get_conversation_metadata_dir(self) -> str: + return CONVERSATION_BASE_DIR def get_conversation_metadata_filename(self, conversation_id: str) -> str: return get_conversation_metadata_filename(conversation_id) diff --git a/tests/unit/test_conversation.py b/tests/unit/test_conversation.py index 73100a8a6d57..69c2fe135f44 100644 --- a/tests/unit/test_conversation.py +++ b/tests/unit/test_conversation.py @@ -6,7 +6,7 @@ import pytest from openhands.server.data_models.conversation_info import ConversationInfo -from openhands.server.data_models.conversation_result_set import ConversationResultSet +from openhands.server.data_models.conversation_info_result_set import ConversationResultSet from openhands.server.data_models.conversation_status import ConversationStatus from openhands.server.routes.new_conversation import ( get_conversation, From bdd23daeef4cde0fc42ed99a3572cb1b7818ab8d Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Tue, 31 Dec 2024 15:19:16 -0700 Subject: [PATCH 31/56] WIP --- tests/unit/test_conversation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_conversation.py b/tests/unit/test_conversation.py index 69c2fe135f44..1c5900f0e968 100644 --- a/tests/unit/test_conversation.py +++ b/tests/unit/test_conversation.py @@ -6,7 +6,7 @@ import pytest from openhands.server.data_models.conversation_info import ConversationInfo -from openhands.server.data_models.conversation_info_result_set import ConversationResultSet +from openhands.server.data_models.conversation_info_result_set import ConversationInfoResultSet from openhands.server.data_models.conversation_status import ConversationStatus from openhands.server.routes.new_conversation import ( get_conversation, @@ -48,7 +48,7 @@ def _patch_store(): async def test_search_conversations(): with _patch_store(): result_set = await search_conversations() - expected = ConversationResultSet( + expected = ConversationInfoResultSet( results=[ ConversationInfo( id='some_conversation_id', From 0e03376b917aec5f95c391b6a46d116eba3a8013 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Tue, 31 Dec 2024 15:34:18 -0700 Subject: [PATCH 32/56] Test fixes --- openhands/server/routes/new_conversation.py | 36 ++++++++++++------- .../conversation/conversation_store.py | 6 ++-- .../conversation/file_conversation_store.py | 15 +++++--- tests/unit/test_conversation.py | 12 ++++--- 4 files changed, 45 insertions(+), 24 deletions(-) diff --git a/openhands/server/routes/new_conversation.py b/openhands/server/routes/new_conversation.py index 974da5c11a83..b0e7419cc2c0 100644 --- a/openhands/server/routes/new_conversation.py +++ b/openhands/server/routes/new_conversation.py @@ -10,7 +10,9 @@ from openhands.core.logger import openhands_logger as logger from openhands.server.data_models.conversation_info import ConversationInfo from openhands.server.data_models.conversation_metadata import ConversationMetadata -from openhands.server.data_models.conversation_info_result_set import ConversationInfoResultSet +from openhands.server.data_models.conversation_info_result_set import ( + ConversationInfoResultSet, +) from openhands.server.data_models.conversation_status import ConversationStatus from openhands.server.routes.settings import ConversationStoreImpl, SettingsStoreImpl from openhands.server.session.conversation_init_data import ConversationInitData @@ -94,17 +96,22 @@ async def search_conversations( github_token = getattr(request.state, 'github_token', '') or '' conversation_store = await ConversationStoreImpl.get_instance(config, github_token) conversation_metadata_result_set = await conversation_store.search(page_id, limit) - conversation_ids = set(conversation.conversation_id for conversation in conversation_metadata_result_set.results) - running_conversations = await session_manager.get_agent_loop_running(set(conversation_ids)) + conversation_ids = set( + conversation.conversation_id + for conversation in conversation_metadata_result_set.results + ) + running_conversations = await session_manager.get_agent_loop_running( + set(conversation_ids) + ) result = ConversationInfoResultSet( results=await wait_all( _get_conversation_info( conversation=conversation, - is_running=conversation.conversation_id in running_conversations + is_running=conversation.conversation_id in running_conversations, ) for conversation in conversation_metadata_result_set.results ), - next_page_id=conversation_metadata_result_set.next_page_id + next_page_id=conversation_metadata_result_set.next_page_id, ) return result @@ -139,13 +146,18 @@ async def _get_conversation_info( @app.get('/conversations/{conversation_id}') -async def get_conversation(conversation_id: str, request: Request) -> ConversationInfo | None: - file_store = session_manager.file_store - is_running = await session_manager.is_agent_loop_running(conversation_id) - conversation_info = await _get_conversation_info( - conversation_id, is_running, file_store, request - ) - return conversation_info +async def get_conversation( + conversation_id: str, request: Request +) -> ConversationInfo | None: + github_token = getattr(request.state, 'github_token', '') or '' + conversation_store = await ConversationStoreImpl.get_instance(config, github_token) + try: + metadata = await conversation_store.get_metadata(conversation_id) + is_running = await session_manager.is_agent_loop_running(conversation_id) + conversation_info = await _get_conversation_info(metadata, is_running) + return conversation_info + except FileNotFoundError: + return None @app.post('/conversations/{conversation_id}') diff --git a/openhands/storage/conversation/conversation_store.py b/openhands/storage/conversation/conversation_store.py index bf479fc1e7a3..7e7c822617d0 100644 --- a/openhands/storage/conversation/conversation_store.py +++ b/openhands/storage/conversation/conversation_store.py @@ -4,7 +4,9 @@ from openhands.core.config.app_config import AppConfig from openhands.server.data_models.conversation_metadata import ConversationMetadata -from openhands.server.data_models.conversation_metadata_result_set import ConversationMetadataResultSet +from openhands.server.data_models.conversation_metadata_result_set import ( + ConversationMetadataResultSet, +) class ConversationStore(ABC): @@ -30,7 +32,7 @@ async def exists(self, conversation_id: str) -> bool: @abstractmethod async def search( - self, + self, page_id: str | None = None, limit: int = 20, ) -> ConversationMetadataResultSet: diff --git a/openhands/storage/conversation/file_conversation_store.py b/openhands/storage/conversation/file_conversation_store.py index f412617fcbc4..d9dfc8a6adfb 100644 --- a/openhands/storage/conversation/file_conversation_store.py +++ b/openhands/storage/conversation/file_conversation_store.py @@ -5,12 +5,17 @@ from openhands.core.config.app_config import AppConfig from openhands.core.logger import openhands_logger as logger -from openhands.server.data_models.conversation_metadata_result_set import ConversationMetadataResultSet +from openhands.server.data_models.conversation_metadata_result_set import ( + ConversationMetadataResultSet, +) from openhands.storage import get_file_store from openhands.storage.conversation.conversation_store import ConversationStore from openhands.server.data_models.conversation_metadata import ConversationMetadata from openhands.storage.files import FileStore -from openhands.storage.locations import CONVERSATION_BASE_DIR, get_conversation_metadata_filename +from openhands.storage.locations import ( + CONVERSATION_BASE_DIR, + get_conversation_metadata_filename, +) from openhands.utils.async_utils import call_sync_from_async from openhands.utils.search_utils import offset_to_page_id, page_id_to_offset @@ -40,9 +45,9 @@ async def exists(self, conversation_id: str) -> bool: return True except FileNotFoundError: return False - + async def search( - self, + self, page_id: str | None = None, limit: int = 20, ) -> ConversationMetadataResultSet: @@ -69,7 +74,7 @@ async def search( ) next_page_id = offset_to_page_id(end, end < num_conversations) return ConversationMetadataResultSet(conversations, next_page_id) - + def get_conversation_metadata_dir(self) -> str: return CONVERSATION_BASE_DIR diff --git a/tests/unit/test_conversation.py b/tests/unit/test_conversation.py index 1c5900f0e968..c3ee4ae1fe8c 100644 --- a/tests/unit/test_conversation.py +++ b/tests/unit/test_conversation.py @@ -6,7 +6,9 @@ import pytest from openhands.server.data_models.conversation_info import ConversationInfo -from openhands.server.data_models.conversation_info_result_set import ConversationInfoResultSet +from openhands.server.data_models.conversation_info_result_set import ( + ConversationInfoResultSet, +) from openhands.server.data_models.conversation_status import ConversationStatus from openhands.server.routes.new_conversation import ( get_conversation, @@ -34,7 +36,7 @@ def _patch_store(): json.dumps({'timestamp': '2025-01-01T00:00:00'}), ) with patch( - 'openhands.storage.conversation.conversation_store.get_file_store', + 'openhands.storage.conversation.file_conversation_store.get_file_store', MagicMock(return_value=file_store), ): with patch( @@ -47,7 +49,7 @@ def _patch_store(): @pytest.mark.asyncio async def test_search_conversations(): with _patch_store(): - result_set = await search_conversations() + result_set = await search_conversations(MagicMock(state=MagicMock(github_token=''))) expected = ConversationInfoResultSet( results=[ ConversationInfo( @@ -65,7 +67,7 @@ async def test_search_conversations(): @pytest.mark.asyncio async def test_get_conversation(): with _patch_store(): - conversation = await get_conversation('some_conversation_id') + conversation = await get_conversation('some_conversation_id', MagicMock(state=MagicMock(github_token=''))) expected = ConversationInfo( id='some_conversation_id', title='Some Conversation', @@ -82,4 +84,4 @@ async def test_get_missing_conversation(): 'openhands.server.routes.new_conversation.session_manager.file_store', InMemoryFileStore({}), ): - assert await get_conversation('no_such_conversation') is None + assert await get_conversation('no_such_conversation', MagicMock(state=MagicMock(github_token=''))) is None From d3d1b28e3a6300736cbde257dee8875bf2647eef Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Tue, 31 Dec 2024 15:34:41 -0700 Subject: [PATCH 33/56] Ruff --- tests/unit/test_conversation.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_conversation.py b/tests/unit/test_conversation.py index c3ee4ae1fe8c..f458cb09d170 100644 --- a/tests/unit/test_conversation.py +++ b/tests/unit/test_conversation.py @@ -49,7 +49,9 @@ def _patch_store(): @pytest.mark.asyncio async def test_search_conversations(): with _patch_store(): - result_set = await search_conversations(MagicMock(state=MagicMock(github_token=''))) + result_set = await search_conversations( + MagicMock(state=MagicMock(github_token='')) + ) expected = ConversationInfoResultSet( results=[ ConversationInfo( @@ -67,7 +69,9 @@ async def test_search_conversations(): @pytest.mark.asyncio async def test_get_conversation(): with _patch_store(): - conversation = await get_conversation('some_conversation_id', MagicMock(state=MagicMock(github_token=''))) + conversation = await get_conversation( + 'some_conversation_id', MagicMock(state=MagicMock(github_token='')) + ) expected = ConversationInfo( id='some_conversation_id', title='Some Conversation', @@ -84,4 +88,9 @@ async def test_get_missing_conversation(): 'openhands.server.routes.new_conversation.session_manager.file_store', InMemoryFileStore({}), ): - assert await get_conversation('no_such_conversation', MagicMock(state=MagicMock(github_token=''))) is None + assert ( + await get_conversation( + 'no_such_conversation', MagicMock(state=MagicMock(github_token='')) + ) + is None + ) From c5f8850dc108be96a8cc03decea86cb68b43d769 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Tue, 31 Dec 2024 15:36:30 -0700 Subject: [PATCH 34/56] Lint fix --- openhands/storage/conversation/file_conversation_store.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openhands/storage/conversation/file_conversation_store.py b/openhands/storage/conversation/file_conversation_store.py index d9dfc8a6adfb..1215fa021eb7 100644 --- a/openhands/storage/conversation/file_conversation_store.py +++ b/openhands/storage/conversation/file_conversation_store.py @@ -51,7 +51,7 @@ async def search( page_id: str | None = None, limit: int = 20, ) -> ConversationMetadataResultSet: - conversations = [] + conversations: list[ConversationMetadata] = [] metadata_dir = self.get_conversation_metadata_dir() conversation_ids = [ path.split('/')[-2] From 3f4e87fd096ed17af0206f989ff69a587424695f Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Thu, 2 Jan 2025 07:00:13 -0700 Subject: [PATCH 35/56] Handled case where there are no conversations --- .../storage/conversation/file_conversation_store.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/openhands/storage/conversation/file_conversation_store.py b/openhands/storage/conversation/file_conversation_store.py index 1215fa021eb7..4654b5af55c0 100644 --- a/openhands/storage/conversation/file_conversation_store.py +++ b/openhands/storage/conversation/file_conversation_store.py @@ -53,11 +53,14 @@ async def search( ) -> ConversationMetadataResultSet: conversations: list[ConversationMetadata] = [] metadata_dir = self.get_conversation_metadata_dir() - conversation_ids = [ - path.split('/')[-2] - for path in self.file_store.list(metadata_dir) - if not path.startswith(f'{metadata_dir}/.') - ] + try: + conversation_ids = [ + path.split('/')[-2] + for path in self.file_store.list(metadata_dir) + if not path.startswith(f'{metadata_dir}/.') + ] + except FileNotFoundError: + return ConversationMetadataResultSet([]) num_conversations = len(conversation_ids) start = page_id_to_offset(page_id) end = min(limit + start, num_conversations) From cf1d1480dc79659d2d0e6db65b5ccc18828973dc Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Thu, 2 Jan 2025 07:05:37 -0700 Subject: [PATCH 36/56] Implemented default title --- openhands/server/data_models/conversation_info.py | 2 +- openhands/server/routes/new_conversation.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/openhands/server/data_models/conversation_info.py b/openhands/server/data_models/conversation_info.py index 1d3ab6931d5b..51266a5760ee 100644 --- a/openhands/server/data_models/conversation_info.py +++ b/openhands/server/data_models/conversation_info.py @@ -9,7 +9,7 @@ class ConversationInfo: """Information about a conversation""" id: str - title: str | None = None + title: str last_updated_at: datetime | None = None status: ConversationStatus = ConversationStatus.STOPPED selected_repository: str | None = None diff --git a/openhands/server/routes/new_conversation.py b/openhands/server/routes/new_conversation.py index b0e7419cc2c0..b924b23dd09f 100644 --- a/openhands/server/routes/new_conversation.py +++ b/openhands/server/routes/new_conversation.py @@ -127,6 +127,9 @@ async def _get_conversation_info( events = sorted(events) event_path = events[-1] event = json.loads(file_store.read(event_path)) + title = conversation.title + if not title: + title = f"Conversation {conversation.conversation_id}" return ConversationInfo( id=conversation.conversation_id, title=conversation.title, From 2d667c340245924d5540d20cacdc3ec33e264de1 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Thu, 2 Jan 2025 07:12:49 -0700 Subject: [PATCH 37/56] Moved as suggested --- openhands/server/routes/new_conversation.py | 43 ++------------------- openhands/utils/conversation_utils.py | 40 +++++++++++++++++++ 2 files changed, 43 insertions(+), 40 deletions(-) create mode 100644 openhands/utils/conversation_utils.py diff --git a/openhands/server/routes/new_conversation.py b/openhands/server/routes/new_conversation.py index b924b23dd09f..d0c2d0114416 100644 --- a/openhands/server/routes/new_conversation.py +++ b/openhands/server/routes/new_conversation.py @@ -1,6 +1,4 @@ -import json import uuid -from datetime import datetime from fastapi import APIRouter, Request from fastapi.responses import JSONResponse @@ -13,14 +11,11 @@ from openhands.server.data_models.conversation_info_result_set import ( ConversationInfoResultSet, ) -from openhands.server.data_models.conversation_status import ConversationStatus from openhands.server.routes.settings import ConversationStoreImpl, SettingsStoreImpl from openhands.server.session.conversation_init_data import ConversationInitData from openhands.server.shared import config, session_manager -from openhands.storage.files import FileStore -from openhands.storage.locations import get_conversation_events_dir from openhands.utils.async_utils import call_sync_from_async, wait_all -from openhands.utils.search_utils import offset_to_page_id, page_id_to_offset +from openhands.utils.conversation_utils import get_conversation_info app = APIRouter(prefix='/api') @@ -105,7 +100,7 @@ async def search_conversations( ) result = ConversationInfoResultSet( results=await wait_all( - _get_conversation_info( + get_conversation_info( conversation=conversation, is_running=conversation.conversation_id in running_conversations, ) @@ -116,38 +111,6 @@ async def search_conversations( return result -async def _get_conversation_info( - conversation: ConversationMetadata, - is_running: bool, -) -> ConversationInfo | None: - try: - file_store = session_manager.file_store - events_dir = get_conversation_events_dir(conversation.conversation_id) - events = file_store.list(events_dir) - events = sorted(events) - event_path = events[-1] - event = json.loads(file_store.read(event_path)) - title = conversation.title - if not title: - title = f"Conversation {conversation.conversation_id}" - return ConversationInfo( - id=conversation.conversation_id, - title=conversation.title, - last_updated_at=datetime.fromisoformat(event.get('timestamp')), - selected_repository=conversation.selected_repository, - status=ConversationStatus.RUNNING - if is_running - else ConversationStatus.STOPPED, - ) - except Exception: # type: ignore - logger.warning( - f'Error loading conversation: {conversation.conversation_id}', - exc_info=True, - stack_info=True, - ) - return None - - @app.get('/conversations/{conversation_id}') async def get_conversation( conversation_id: str, request: Request @@ -157,7 +120,7 @@ async def get_conversation( try: metadata = await conversation_store.get_metadata(conversation_id) is_running = await session_manager.is_agent_loop_running(conversation_id) - conversation_info = await _get_conversation_info(metadata, is_running) + conversation_info = await get_conversation_info(metadata, is_running) return conversation_info except FileNotFoundError: return None diff --git a/openhands/utils/conversation_utils.py b/openhands/utils/conversation_utils.py new file mode 100644 index 000000000000..d65790b0364e --- /dev/null +++ b/openhands/utils/conversation_utils.py @@ -0,0 +1,40 @@ +from datetime import datetime +import json +from openhands.core.logger import openhands_logger as logger +from openhands.server.data_models.conversation_info import ConversationInfo +from openhands.server.data_models.conversation_metadata import ConversationMetadata +from openhands.server.data_models.conversation_status import ConversationStatus +from openhands.server.shared import session_manager +from openhands.storage.locations import get_conversation_events_dir + + +async def get_conversation_info( + conversation: ConversationMetadata, + is_running: bool, +) -> ConversationInfo | None: + try: + file_store = session_manager.file_store + events_dir = get_conversation_events_dir(conversation.conversation_id) + events = file_store.list(events_dir) + events = sorted(events) + event_path = events[-1] + event = json.loads(file_store.read(event_path)) + title = conversation.title + if not title: + title = f'Conversation {conversation.conversation_id}' + return ConversationInfo( + id=conversation.conversation_id, + title=conversation.title, + last_updated_at=datetime.fromisoformat(event.get('timestamp')), + selected_repository=conversation.selected_repository, + status=ConversationStatus.RUNNING + if is_running + else ConversationStatus.STOPPED, + ) + except Exception: # type: ignore + logger.warning( + f'Error loading conversation: {conversation.conversation_id}', + exc_info=True, + stack_info=True, + ) + return None From 215c633f46ff0f590a764726d3e71547634575a8 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Thu, 2 Jan 2025 07:14:01 -0700 Subject: [PATCH 38/56] Using put as post is for create --- openhands/server/routes/new_conversation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openhands/server/routes/new_conversation.py b/openhands/server/routes/new_conversation.py index d0c2d0114416..bf5249f77451 100644 --- a/openhands/server/routes/new_conversation.py +++ b/openhands/server/routes/new_conversation.py @@ -126,7 +126,7 @@ async def get_conversation( return None -@app.post('/conversations/{conversation_id}') +@app.put('/conversations/{conversation_id}') async def update_conversation( conversation_id: str, title: str, request: Request ) -> bool: From d2a80d92f3f637783c8a9054cfdb07880e2cab13 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Thu, 2 Jan 2025 07:16:39 -0700 Subject: [PATCH 39/56] Fixed typo --- openhands/utils/conversation_utils.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/openhands/utils/conversation_utils.py b/openhands/utils/conversation_utils.py index d65790b0364e..4fbd5ad03b01 100644 --- a/openhands/utils/conversation_utils.py +++ b/openhands/utils/conversation_utils.py @@ -1,5 +1,6 @@ -from datetime import datetime import json +from datetime import datetime + from openhands.core.logger import openhands_logger as logger from openhands.server.data_models.conversation_info import ConversationInfo from openhands.server.data_models.conversation_metadata import ConversationMetadata @@ -24,7 +25,7 @@ async def get_conversation_info( title = f'Conversation {conversation.conversation_id}' return ConversationInfo( id=conversation.conversation_id, - title=conversation.title, + title=title, last_updated_at=datetime.fromisoformat(event.get('timestamp')), selected_repository=conversation.selected_repository, status=ConversationStatus.RUNNING From 5becacfa01cffa98c9317e0a5639e263433a9203 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Thu, 2 Jan 2025 07:22:35 -0700 Subject: [PATCH 40/56] Added test for update conversation --- tests/unit/test_conversation.py | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/tests/unit/test_conversation.py b/tests/unit/test_conversation.py index f458cb09d170..fc2c40873d6d 100644 --- a/tests/unit/test_conversation.py +++ b/tests/unit/test_conversation.py @@ -13,6 +13,7 @@ from openhands.server.routes.new_conversation import ( get_conversation, search_conversations, + update_conversation, ) from openhands.storage.memory import InMemoryFileStore @@ -84,13 +85,31 @@ async def test_get_conversation(): @pytest.mark.asyncio async def test_get_missing_conversation(): - with patch( - 'openhands.server.routes.new_conversation.session_manager.file_store', - InMemoryFileStore({}), - ): + with _patch_store(): assert ( await get_conversation( 'no_such_conversation', MagicMock(state=MagicMock(github_token='')) ) is None ) + + +@pytest.mark.asyncio +async def test_update_conversation(): + with _patch_store(): + await update_conversation( + 'some_conversation_id', + 'New Title', + MagicMock(state=MagicMock(github_token='')), + ) + conversation = await get_conversation( + 'some_conversation_id', MagicMock(state=MagicMock(github_token='')) + ) + expected = ConversationInfo( + id='some_conversation_id', + title='New Title', + last_updated_at=datetime.fromisoformat('2025-01-01T00:00:00'), + status=ConversationStatus.STOPPED, + selected_repository='foobar', + ) + assert conversation == expected From ea5f583dd5b9965e3e87cef7af46eb7b5ff7c283 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Thu, 2 Jan 2025 07:28:55 -0700 Subject: [PATCH 41/56] Moved models as suggested by Enyst --- openhands/server/routes/new_conversation.py | 6 +++--- openhands/storage/conversation/conversation_store.py | 4 ++-- openhands/storage/conversation/file_conversation_store.py | 4 ++-- .../{server => storage}/data_models/conversation_info.py | 2 +- .../data_models/conversation_info_result_set.py | 2 +- .../data_models/conversation_metadata.py | 0 .../data_models/conversation_metadata_result_set.py | 2 +- .../{server => storage}/data_models/conversation_status.py | 0 openhands/utils/conversation_utils.py | 6 +++--- tests/unit/test_conversation.py | 6 +++--- 10 files changed, 16 insertions(+), 16 deletions(-) rename openhands/{server => storage}/data_models/conversation_info.py (79%) rename openhands/{server => storage}/data_models/conversation_info_result_set.py (71%) rename openhands/{server => storage}/data_models/conversation_metadata.py (100%) rename openhands/{server => storage}/data_models/conversation_metadata_result_set.py (69%) rename openhands/{server => storage}/data_models/conversation_status.py (100%) diff --git a/openhands/server/routes/new_conversation.py b/openhands/server/routes/new_conversation.py index bf5249f77451..30bb4a1f707b 100644 --- a/openhands/server/routes/new_conversation.py +++ b/openhands/server/routes/new_conversation.py @@ -6,9 +6,9 @@ from pydantic import BaseModel from openhands.core.logger import openhands_logger as logger -from openhands.server.data_models.conversation_info import ConversationInfo -from openhands.server.data_models.conversation_metadata import ConversationMetadata -from openhands.server.data_models.conversation_info_result_set import ( +from openhands.storage.data_models.conversation_info import ConversationInfo +from openhands.storage.data_models.conversation_metadata import ConversationMetadata +from openhands.storage.data_models.conversation_info_result_set import ( ConversationInfoResultSet, ) from openhands.server.routes.settings import ConversationStoreImpl, SettingsStoreImpl diff --git a/openhands/storage/conversation/conversation_store.py b/openhands/storage/conversation/conversation_store.py index 7e7c822617d0..2a09322574bf 100644 --- a/openhands/storage/conversation/conversation_store.py +++ b/openhands/storage/conversation/conversation_store.py @@ -3,8 +3,8 @@ from abc import ABC, abstractmethod from openhands.core.config.app_config import AppConfig -from openhands.server.data_models.conversation_metadata import ConversationMetadata -from openhands.server.data_models.conversation_metadata_result_set import ( +from openhands.storage.data_models.conversation_metadata import ConversationMetadata +from openhands.storage.data_models.conversation_metadata_result_set import ( ConversationMetadataResultSet, ) diff --git a/openhands/storage/conversation/file_conversation_store.py b/openhands/storage/conversation/file_conversation_store.py index 4654b5af55c0..95260690e7b2 100644 --- a/openhands/storage/conversation/file_conversation_store.py +++ b/openhands/storage/conversation/file_conversation_store.py @@ -5,12 +5,12 @@ from openhands.core.config.app_config import AppConfig from openhands.core.logger import openhands_logger as logger -from openhands.server.data_models.conversation_metadata_result_set import ( +from openhands.storage.data_models.conversation_metadata_result_set import ( ConversationMetadataResultSet, ) from openhands.storage import get_file_store from openhands.storage.conversation.conversation_store import ConversationStore -from openhands.server.data_models.conversation_metadata import ConversationMetadata +from openhands.storage.data_models.conversation_metadata import ConversationMetadata from openhands.storage.files import FileStore from openhands.storage.locations import ( CONVERSATION_BASE_DIR, diff --git a/openhands/server/data_models/conversation_info.py b/openhands/storage/data_models/conversation_info.py similarity index 79% rename from openhands/server/data_models/conversation_info.py rename to openhands/storage/data_models/conversation_info.py index 51266a5760ee..5e94fe310e9a 100644 --- a/openhands/server/data_models/conversation_info.py +++ b/openhands/storage/data_models/conversation_info.py @@ -1,7 +1,7 @@ from dataclasses import dataclass from datetime import datetime -from openhands.server.data_models.conversation_status import ConversationStatus +from openhands.storage.data_models.conversation_status import ConversationStatus @dataclass diff --git a/openhands/server/data_models/conversation_info_result_set.py b/openhands/storage/data_models/conversation_info_result_set.py similarity index 71% rename from openhands/server/data_models/conversation_info_result_set.py rename to openhands/storage/data_models/conversation_info_result_set.py index 5f91f5f531b3..b153baf1a290 100644 --- a/openhands/server/data_models/conversation_info_result_set.py +++ b/openhands/storage/data_models/conversation_info_result_set.py @@ -1,6 +1,6 @@ from dataclasses import dataclass, field -from openhands.server.data_models.conversation_info import ConversationInfo +from openhands.storage.data_models.conversation_info import ConversationInfo @dataclass diff --git a/openhands/server/data_models/conversation_metadata.py b/openhands/storage/data_models/conversation_metadata.py similarity index 100% rename from openhands/server/data_models/conversation_metadata.py rename to openhands/storage/data_models/conversation_metadata.py diff --git a/openhands/server/data_models/conversation_metadata_result_set.py b/openhands/storage/data_models/conversation_metadata_result_set.py similarity index 69% rename from openhands/server/data_models/conversation_metadata_result_set.py rename to openhands/storage/data_models/conversation_metadata_result_set.py index 74cfc6c8853a..6f93827258c3 100644 --- a/openhands/server/data_models/conversation_metadata_result_set.py +++ b/openhands/storage/data_models/conversation_metadata_result_set.py @@ -1,6 +1,6 @@ from dataclasses import dataclass, field -from openhands.server.data_models.conversation_metadata import ConversationMetadata +from openhands.storage.data_models.conversation_metadata import ConversationMetadata @dataclass diff --git a/openhands/server/data_models/conversation_status.py b/openhands/storage/data_models/conversation_status.py similarity index 100% rename from openhands/server/data_models/conversation_status.py rename to openhands/storage/data_models/conversation_status.py diff --git a/openhands/utils/conversation_utils.py b/openhands/utils/conversation_utils.py index 4fbd5ad03b01..5875ef227c79 100644 --- a/openhands/utils/conversation_utils.py +++ b/openhands/utils/conversation_utils.py @@ -2,9 +2,9 @@ from datetime import datetime from openhands.core.logger import openhands_logger as logger -from openhands.server.data_models.conversation_info import ConversationInfo -from openhands.server.data_models.conversation_metadata import ConversationMetadata -from openhands.server.data_models.conversation_status import ConversationStatus +from openhands.storage.data_models.conversation_info import ConversationInfo +from openhands.storage.data_models.conversation_metadata import ConversationMetadata +from openhands.storage.data_models.conversation_status import ConversationStatus from openhands.server.shared import session_manager from openhands.storage.locations import get_conversation_events_dir diff --git a/tests/unit/test_conversation.py b/tests/unit/test_conversation.py index fc2c40873d6d..0fdb0e3192ab 100644 --- a/tests/unit/test_conversation.py +++ b/tests/unit/test_conversation.py @@ -5,11 +5,11 @@ import pytest -from openhands.server.data_models.conversation_info import ConversationInfo -from openhands.server.data_models.conversation_info_result_set import ( +from openhands.storage.data_models.conversation_info import ConversationInfo +from openhands.storage.data_models.conversation_info_result_set import ( ConversationInfoResultSet, ) -from openhands.server.data_models.conversation_status import ConversationStatus +from openhands.storage.data_models.conversation_status import ConversationStatus from openhands.server.routes.new_conversation import ( get_conversation, search_conversations, From c89f5f76d3eb56b20b47a461c6ed08aa73884639 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Thu, 2 Jan 2025 07:32:03 -0700 Subject: [PATCH 42/56] Renamed file for clarity --- openhands/server/app.py | 2 +- .../routes/{new_conversation.py => manage_conversations.py} | 0 tests/unit/test_conversation.py | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename openhands/server/routes/{new_conversation.py => manage_conversations.py} (100%) diff --git a/openhands/server/app.py b/openhands/server/app.py index 66dd11ca8a84..2efd9055fef5 100644 --- a/openhands/server/app.py +++ b/openhands/server/app.py @@ -20,7 +20,7 @@ from openhands.server.routes.feedback import app as feedback_api_router from openhands.server.routes.files import app as files_api_router from openhands.server.routes.github import app as github_api_router -from openhands.server.routes.new_conversation import app as new_conversation_api_router +from openhands.server.routes.manage_conversations import app as new_conversation_api_router from openhands.server.routes.public import app as public_api_router from openhands.server.routes.security import app as security_api_router from openhands.server.routes.settings import app as settings_router diff --git a/openhands/server/routes/new_conversation.py b/openhands/server/routes/manage_conversations.py similarity index 100% rename from openhands/server/routes/new_conversation.py rename to openhands/server/routes/manage_conversations.py diff --git a/tests/unit/test_conversation.py b/tests/unit/test_conversation.py index 0fdb0e3192ab..6af40661b396 100644 --- a/tests/unit/test_conversation.py +++ b/tests/unit/test_conversation.py @@ -10,7 +10,7 @@ ConversationInfoResultSet, ) from openhands.storage.data_models.conversation_status import ConversationStatus -from openhands.server.routes.new_conversation import ( +from openhands.server.routes.manage_conversations import ( get_conversation, search_conversations, update_conversation, From ba67047bb8879fdfe60181b9f0cec544ecd09d69 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Thu, 2 Jan 2025 07:41:44 -0700 Subject: [PATCH 43/56] Ruff --- openhands/utils/conversation_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openhands/utils/conversation_utils.py b/openhands/utils/conversation_utils.py index 5875ef227c79..3354637abcbf 100644 --- a/openhands/utils/conversation_utils.py +++ b/openhands/utils/conversation_utils.py @@ -2,10 +2,10 @@ from datetime import datetime from openhands.core.logger import openhands_logger as logger +from openhands.server.shared import session_manager from openhands.storage.data_models.conversation_info import ConversationInfo from openhands.storage.data_models.conversation_metadata import ConversationMetadata from openhands.storage.data_models.conversation_status import ConversationStatus -from openhands.server.shared import session_manager from openhands.storage.locations import get_conversation_events_dir From 03150698272cb8f4faafa53c7ce2114f53cc8557 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Thu, 2 Jan 2025 07:42:08 -0700 Subject: [PATCH 44/56] Ruff --- openhands/server/app.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/openhands/server/app.py b/openhands/server/app.py index 2efd9055fef5..7d4c20bbf2b5 100644 --- a/openhands/server/app.py +++ b/openhands/server/app.py @@ -20,7 +20,9 @@ from openhands.server.routes.feedback import app as feedback_api_router from openhands.server.routes.files import app as files_api_router from openhands.server.routes.github import app as github_api_router -from openhands.server.routes.manage_conversations import app as new_conversation_api_router +from openhands.server.routes.manage_conversations import ( + app as new_conversation_api_router, +) from openhands.server.routes.public import app as public_api_router from openhands.server.routes.security import app as security_api_router from openhands.server.routes.settings import app as settings_router From 2b8006062f6b4d5def65ae9707f8e56332c3f67e Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Thu, 2 Jan 2025 07:46:30 -0700 Subject: [PATCH 45/56] Ruff --- tests/unit/test_conversation.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/unit/test_conversation.py b/tests/unit/test_conversation.py index 6af40661b396..2de982185b81 100644 --- a/tests/unit/test_conversation.py +++ b/tests/unit/test_conversation.py @@ -5,16 +5,16 @@ import pytest -from openhands.storage.data_models.conversation_info import ConversationInfo -from openhands.storage.data_models.conversation_info_result_set import ( - ConversationInfoResultSet, -) -from openhands.storage.data_models.conversation_status import ConversationStatus from openhands.server.routes.manage_conversations import ( get_conversation, search_conversations, update_conversation, ) +from openhands.storage.data_models.conversation_info import ConversationInfo +from openhands.storage.data_models.conversation_info_result_set import ( + ConversationInfoResultSet, +) +from openhands.storage.data_models.conversation_status import ConversationStatus from openhands.storage.memory import InMemoryFileStore From 46260e62dbdb37244550a1cc31d09c51d2fe6bb4 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Thu, 2 Jan 2025 07:54:56 -0700 Subject: [PATCH 46/56] Fixed broken tests --- openhands/server/app.py | 4 ++-- tests/unit/test_conversation.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/openhands/server/app.py b/openhands/server/app.py index 7d4c20bbf2b5..1ece8476151a 100644 --- a/openhands/server/app.py +++ b/openhands/server/app.py @@ -21,7 +21,7 @@ from openhands.server.routes.files import app as files_api_router from openhands.server.routes.github import app as github_api_router from openhands.server.routes.manage_conversations import ( - app as new_conversation_api_router, + app as manage_conversation_api_router, ) from openhands.server.routes.public import app as public_api_router from openhands.server.routes.security import app as security_api_router @@ -60,7 +60,7 @@ async def health(): app.include_router(security_api_router) app.include_router(feedback_api_router) app.include_router(conversation_api_router) -app.include_router(new_conversation_api_router) +app.include_router(manage_conversation_api_router) app.include_router(settings_router) app.include_router(github_api_router) diff --git a/tests/unit/test_conversation.py b/tests/unit/test_conversation.py index 2de982185b81..a07ae7f361a7 100644 --- a/tests/unit/test_conversation.py +++ b/tests/unit/test_conversation.py @@ -41,7 +41,7 @@ def _patch_store(): MagicMock(return_value=file_store), ): with patch( - 'openhands.server.routes.new_conversation.session_manager.file_store', + 'openhands.server.routes.manage_conversations.session_manager.file_store', file_store, ): yield From 92d772635b2964d55ec7097d1abf9012cdfe33bb Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Thu, 2 Jan 2025 11:23:13 -0700 Subject: [PATCH 47/56] Now updating conversation metadata as events come in --- .../server/routes/manage_conversations.py | 17 ++++++++-- .../conversation/file_conversation_store.py | 9 +++-- .../data_models/conversation_metadata.py | 4 ++- openhands/utils/conversation_utils.py | 34 ++++++++++++++----- 4 files changed, 51 insertions(+), 13 deletions(-) diff --git a/openhands/server/routes/manage_conversations.py b/openhands/server/routes/manage_conversations.py index 30bb4a1f707b..dfdd29a57448 100644 --- a/openhands/server/routes/manage_conversations.py +++ b/openhands/server/routes/manage_conversations.py @@ -6,6 +6,7 @@ from pydantic import BaseModel from openhands.core.logger import openhands_logger as logger +from openhands.events.stream import EventStreamSubscriber from openhands.storage.data_models.conversation_info import ConversationInfo from openhands.storage.data_models.conversation_metadata import ConversationMetadata from openhands.storage.data_models.conversation_info_result_set import ( @@ -15,9 +16,13 @@ from openhands.server.session.conversation_init_data import ConversationInitData from openhands.server.shared import config, session_manager from openhands.utils.async_utils import call_sync_from_async, wait_all -from openhands.utils.conversation_utils import get_conversation_info +from openhands.utils.conversation_utils import ( + create_conversation_update_callback, + get_conversation_info, +) app = APIRouter(prefix='/api') +UPDATED_AT_CALLBACK_ID = 'updated_at_callback_id' class InitSessionRequest(BaseModel): @@ -75,9 +80,17 @@ async def new_conversation(request: Request, data: InitSessionRequest): ) logger.info(f'Starting agent loop for conversation {conversation_id}') - await session_manager.maybe_start_agent_loop( + event_stream = await session_manager.maybe_start_agent_loop( conversation_id, conversation_init_data ) + try: + event_stream.subscribe( + EventStreamSubscriber.SERVER, + create_conversation_update_callback(data.github_token, conversation_id), + UPDATED_AT_CALLBACK_ID, + ) + except ValueError: + pass # Already subscribed - take no action logger.info(f'Finished initializing conversation {conversation_id}') return JSONResponse(content={'status': 'ok', 'conversation_id': conversation_id}) diff --git a/openhands/storage/conversation/file_conversation_store.py b/openhands/storage/conversation/file_conversation_store.py index 95260690e7b2..29f350dacb5e 100644 --- a/openhands/storage/conversation/file_conversation_store.py +++ b/openhands/storage/conversation/file_conversation_store.py @@ -3,6 +3,8 @@ import json from dataclasses import dataclass +from pydantic import TypeAdapter + from openhands.core.config.app_config import AppConfig from openhands.core.logger import openhands_logger as logger from openhands.storage.data_models.conversation_metadata_result_set import ( @@ -19,20 +21,23 @@ from openhands.utils.async_utils import call_sync_from_async from openhands.utils.search_utils import offset_to_page_id, page_id_to_offset +conversation_metadata_type_adapter = TypeAdapter(ConversationMetadata) + @dataclass class FileConversationStore(ConversationStore): file_store: FileStore async def save_metadata(self, metadata: ConversationMetadata): - json_str = json.dumps(metadata.__dict__) + json_str = conversation_metadata_type_adapter.dump_json(metadata) path = self.get_conversation_metadata_filename(metadata.conversation_id) await call_sync_from_async(self.file_store.write, path, json_str) async def get_metadata(self, conversation_id: str) -> ConversationMetadata: path = self.get_conversation_metadata_filename(conversation_id) json_str = await call_sync_from_async(self.file_store.read, path) - return ConversationMetadata(**json.loads(json_str)) + result = conversation_metadata_type_adapter.validate_json(json_str) + return result async def delete_metadata(self, conversation_id: str) -> None: path = self.get_conversation_metadata_filename(conversation_id) diff --git a/openhands/storage/data_models/conversation_metadata.py b/openhands/storage/data_models/conversation_metadata.py index fbe6ef2535e8..21c84be99eba 100644 --- a/openhands/storage/data_models/conversation_metadata.py +++ b/openhands/storage/data_models/conversation_metadata.py @@ -1,9 +1,11 @@ from dataclasses import dataclass +from datetime import datetime @dataclass class ConversationMetadata: conversation_id: str - github_user_id: str + github_user_id: int | str selected_repository: str | None title: str | None = None + last_updated_at: datetime | None = None diff --git a/openhands/utils/conversation_utils.py b/openhands/utils/conversation_utils.py index 3354637abcbf..cab9f17bfdcb 100644 --- a/openhands/utils/conversation_utils.py +++ b/openhands/utils/conversation_utils.py @@ -1,12 +1,15 @@ import json from datetime import datetime +from typing import Callable from openhands.core.logger import openhands_logger as logger -from openhands.server.shared import session_manager +from openhands.server.routes.settings import ConversationStoreImpl +from openhands.server.shared import config, session_manager from openhands.storage.data_models.conversation_info import ConversationInfo from openhands.storage.data_models.conversation_metadata import ConversationMetadata from openhands.storage.data_models.conversation_status import ConversationStatus from openhands.storage.locations import get_conversation_events_dir +from openhands.utils.async_utils import GENERAL_TIMEOUT, call_async_from_sync async def get_conversation_info( @@ -14,19 +17,13 @@ async def get_conversation_info( is_running: bool, ) -> ConversationInfo | None: try: - file_store = session_manager.file_store - events_dir = get_conversation_events_dir(conversation.conversation_id) - events = file_store.list(events_dir) - events = sorted(events) - event_path = events[-1] - event = json.loads(file_store.read(event_path)) title = conversation.title if not title: title = f'Conversation {conversation.conversation_id}' return ConversationInfo( id=conversation.conversation_id, title=title, - last_updated_at=datetime.fromisoformat(event.get('timestamp')), + last_updated_at=conversation.last_updated_at, selected_repository=conversation.selected_repository, status=ConversationStatus.RUNNING if is_running @@ -39,3 +36,24 @@ async def get_conversation_info( stack_info=True, ) return None + + +def create_conversation_update_callback( + github_token: str, conversation_id: str +) -> Callable: + def callback(*args, **kwargs): + call_async_from_sync( + update_timestamp_for_conversation, + GENERAL_TIMEOUT, + github_token, + conversation_id, + ) + + return callback + + +async def update_timestamp_for_conversation(github_token: str, conversation_id: str): + conversation_store = await ConversationStoreImpl.get_instance(config, github_token) + conversation = await conversation_store.get_metadata(conversation_id) + conversation.last_updated_at = datetime.now() + await conversation_store.save_metadata(conversation) From d3b02e0b0f7abe51ed5fb800f8a5a9199f926928 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Thu, 2 Jan 2025 11:24:41 -0700 Subject: [PATCH 48/56] Lint fixes --- openhands/utils/conversation_utils.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/openhands/utils/conversation_utils.py b/openhands/utils/conversation_utils.py index cab9f17bfdcb..8e736d70a706 100644 --- a/openhands/utils/conversation_utils.py +++ b/openhands/utils/conversation_utils.py @@ -4,11 +4,10 @@ from openhands.core.logger import openhands_logger as logger from openhands.server.routes.settings import ConversationStoreImpl -from openhands.server.shared import config, session_manager +from openhands.server.shared import config from openhands.storage.data_models.conversation_info import ConversationInfo from openhands.storage.data_models.conversation_metadata import ConversationMetadata from openhands.storage.data_models.conversation_status import ConversationStatus -from openhands.storage.locations import get_conversation_events_dir from openhands.utils.async_utils import GENERAL_TIMEOUT, call_async_from_sync From 792b2f98c49eb867df80273d0a9fe6628bfafa5a Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Thu, 2 Jan 2025 11:31:31 -0700 Subject: [PATCH 49/56] Lint fix --- openhands/utils/conversation_utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/openhands/utils/conversation_utils.py b/openhands/utils/conversation_utils.py index 8e736d70a706..cae7ff963863 100644 --- a/openhands/utils/conversation_utils.py +++ b/openhands/utils/conversation_utils.py @@ -1,4 +1,3 @@ -import json from datetime import datetime from typing import Callable From 9cedca17a1eba4370eaa7d123481af0758da871f Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Thu, 2 Jan 2025 11:59:01 -0700 Subject: [PATCH 50/56] Test fixes --- tests/unit/test_conversation.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/unit/test_conversation.py b/tests/unit/test_conversation.py index a07ae7f361a7..73fa533e5aa1 100644 --- a/tests/unit/test_conversation.py +++ b/tests/unit/test_conversation.py @@ -29,13 +29,10 @@ def _patch_store(): 'selected_repository': 'foobar', 'conversation_id': 'some_conversation_id', 'github_user_id': 'github_user', + 'last_updated_at': '2025-01-01T00:00:00', } ), ) - file_store.write( - 'sessions/some_conversation_id/events/0.json', - json.dumps({'timestamp': '2025-01-01T00:00:00'}), - ) with patch( 'openhands.storage.conversation.file_conversation_store.get_file_store', MagicMock(return_value=file_store), From 0290a075ddac0258c4c3cffaf66535e1ad678962 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Thu, 2 Jan 2025 12:07:47 -0700 Subject: [PATCH 51/56] Shorter conversation name --- openhands/utils/conversation_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openhands/utils/conversation_utils.py b/openhands/utils/conversation_utils.py index cae7ff963863..d39d03d84646 100644 --- a/openhands/utils/conversation_utils.py +++ b/openhands/utils/conversation_utils.py @@ -29,7 +29,7 @@ async def get_conversation_info( ) except Exception: # type: ignore logger.warning( - f'Error loading conversation: {conversation.conversation_id}', + f'Error loading conversation: {conversation.conversation_id[:5]}', exc_info=True, stack_info=True, ) From 502f02a2ecfbf4fffcbba73841ea2335ad4eefcb Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Thu, 2 Jan 2025 12:55:58 -0700 Subject: [PATCH 52/56] Less utils --- .../server/routes/manage_conversations.py | 65 +++++++++++++++++-- openhands/utils/conversation_utils.py | 57 ---------------- 2 files changed, 58 insertions(+), 64 deletions(-) delete mode 100644 openhands/utils/conversation_utils.py diff --git a/openhands/server/routes/manage_conversations.py b/openhands/server/routes/manage_conversations.py index dfbfcec44ca9..eaa0be80a007 100644 --- a/openhands/server/routes/manage_conversations.py +++ b/openhands/server/routes/manage_conversations.py @@ -1,4 +1,6 @@ import uuid +from datetime import datetime +from typing import Callable from fastapi import APIRouter, Request from fastapi.responses import JSONResponse @@ -15,10 +17,12 @@ ConversationInfoResultSet, ) from openhands.storage.data_models.conversation_metadata import ConversationMetadata -from openhands.utils.async_utils import call_sync_from_async, wait_all -from openhands.utils.conversation_utils import ( - create_conversation_update_callback, - get_conversation_info, +from openhands.storage.data_models.conversation_status import ConversationStatus +from openhands.utils.async_utils import ( + GENERAL_TIMEOUT, + call_async_from_sync, + call_sync_from_async, + wait_all, ) app = APIRouter(prefix='/api') @@ -86,7 +90,7 @@ async def new_conversation(request: Request, data: InitSessionRequest): try: event_stream.subscribe( EventStreamSubscriber.SERVER, - create_conversation_update_callback( + _create_conversation_update_callback( data.github_token or '', conversation_id ), UPDATED_AT_CALLBACK_ID, @@ -115,7 +119,7 @@ async def search_conversations( ) result = ConversationInfoResultSet( results=await wait_all( - get_conversation_info( + _get_conversation_info( conversation=conversation, is_running=conversation.conversation_id in running_conversations, ) @@ -135,7 +139,7 @@ async def get_conversation( try: metadata = await conversation_store.get_metadata(conversation_id) is_running = await session_manager.is_agent_loop_running(conversation_id) - conversation_info = await get_conversation_info(metadata, is_running) + conversation_info = await _get_conversation_info(metadata, is_running) return conversation_info except FileNotFoundError: return None @@ -171,3 +175,50 @@ async def delete_conversation( return False await conversation_store.delete_metadata(conversation_id) return True + + +async def _get_conversation_info( + conversation: ConversationMetadata, + is_running: bool, +) -> ConversationInfo | None: + try: + title = conversation.title + if not title: + title = f'Conversation {conversation.conversation_id}' + return ConversationInfo( + id=conversation.conversation_id, + title=title, + last_updated_at=conversation.last_updated_at, + selected_repository=conversation.selected_repository, + status=ConversationStatus.RUNNING + if is_running + else ConversationStatus.STOPPED, + ) + except Exception: # type: ignore + logger.warning( + f'Error loading conversation: {conversation.conversation_id[:5]}', + exc_info=True, + stack_info=True, + ) + return None + + +def _create_conversation_update_callback( + github_token: str, conversation_id: str +) -> Callable: + def callback(*args, **kwargs): + call_async_from_sync( + _update_timestamp_for_conversation, + GENERAL_TIMEOUT, + github_token, + conversation_id, + ) + + return callback + + +async def _update_timestamp_for_conversation(github_token: str, conversation_id: str): + conversation_store = await ConversationStoreImpl.get_instance(config, github_token) + conversation = await conversation_store.get_metadata(conversation_id) + conversation.last_updated_at = datetime.now() + await conversation_store.save_metadata(conversation) diff --git a/openhands/utils/conversation_utils.py b/openhands/utils/conversation_utils.py deleted file mode 100644 index d39d03d84646..000000000000 --- a/openhands/utils/conversation_utils.py +++ /dev/null @@ -1,57 +0,0 @@ -from datetime import datetime -from typing import Callable - -from openhands.core.logger import openhands_logger as logger -from openhands.server.routes.settings import ConversationStoreImpl -from openhands.server.shared import config -from openhands.storage.data_models.conversation_info import ConversationInfo -from openhands.storage.data_models.conversation_metadata import ConversationMetadata -from openhands.storage.data_models.conversation_status import ConversationStatus -from openhands.utils.async_utils import GENERAL_TIMEOUT, call_async_from_sync - - -async def get_conversation_info( - conversation: ConversationMetadata, - is_running: bool, -) -> ConversationInfo | None: - try: - title = conversation.title - if not title: - title = f'Conversation {conversation.conversation_id}' - return ConversationInfo( - id=conversation.conversation_id, - title=title, - last_updated_at=conversation.last_updated_at, - selected_repository=conversation.selected_repository, - status=ConversationStatus.RUNNING - if is_running - else ConversationStatus.STOPPED, - ) - except Exception: # type: ignore - logger.warning( - f'Error loading conversation: {conversation.conversation_id[:5]}', - exc_info=True, - stack_info=True, - ) - return None - - -def create_conversation_update_callback( - github_token: str, conversation_id: str -) -> Callable: - def callback(*args, **kwargs): - call_async_from_sync( - update_timestamp_for_conversation, - GENERAL_TIMEOUT, - github_token, - conversation_id, - ) - - return callback - - -async def update_timestamp_for_conversation(github_token: str, conversation_id: str): - conversation_store = await ConversationStoreImpl.get_instance(config, github_token) - conversation = await conversation_store.get_metadata(conversation_id) - conversation.last_updated_at = datetime.now() - await conversation_store.save_metadata(conversation) From ca99db3fbf49f4d233a816af5a00266811d0587c Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Thu, 2 Jan 2025 13:36:19 -0700 Subject: [PATCH 53/56] Using first 5 chars --- openhands/server/routes/manage_conversations.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openhands/server/routes/manage_conversations.py b/openhands/server/routes/manage_conversations.py index eaa0be80a007..7b85efdbca37 100644 --- a/openhands/server/routes/manage_conversations.py +++ b/openhands/server/routes/manage_conversations.py @@ -184,7 +184,7 @@ async def _get_conversation_info( try: title = conversation.title if not title: - title = f'Conversation {conversation.conversation_id}' + title = f'Conversation {conversation.conversation_id[:5]}' return ConversationInfo( id=conversation.conversation_id, title=title, From f4a0645980939cca1035280e46845b12184c8195 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Thu, 2 Jan 2025 13:58:33 -0700 Subject: [PATCH 54/56] Unified API as suggested by Stephan --- openhands/server/routes/manage_conversations.py | 2 +- openhands/storage/data_models/conversation_info.py | 2 +- tests/unit/test_conversation.py | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/openhands/server/routes/manage_conversations.py b/openhands/server/routes/manage_conversations.py index 7b85efdbca37..a3f4af42ddcd 100644 --- a/openhands/server/routes/manage_conversations.py +++ b/openhands/server/routes/manage_conversations.py @@ -186,7 +186,7 @@ async def _get_conversation_info( if not title: title = f'Conversation {conversation.conversation_id[:5]}' return ConversationInfo( - id=conversation.conversation_id, + conversation_id=conversation.conversation_id, title=title, last_updated_at=conversation.last_updated_at, selected_repository=conversation.selected_repository, diff --git a/openhands/storage/data_models/conversation_info.py b/openhands/storage/data_models/conversation_info.py index 5e94fe310e9a..cda7a2653bff 100644 --- a/openhands/storage/data_models/conversation_info.py +++ b/openhands/storage/data_models/conversation_info.py @@ -8,7 +8,7 @@ class ConversationInfo: """Information about a conversation""" - id: str + conversation_id: str title: str last_updated_at: datetime | None = None status: ConversationStatus = ConversationStatus.STOPPED diff --git a/tests/unit/test_conversation.py b/tests/unit/test_conversation.py index 73fa533e5aa1..f6e8bdd384c9 100644 --- a/tests/unit/test_conversation.py +++ b/tests/unit/test_conversation.py @@ -53,7 +53,7 @@ async def test_search_conversations(): expected = ConversationInfoResultSet( results=[ ConversationInfo( - id='some_conversation_id', + conversation_id='some_conversation_id', title='Some Conversation', last_updated_at=datetime.fromisoformat('2025-01-01T00:00:00'), status=ConversationStatus.STOPPED, @@ -71,7 +71,7 @@ async def test_get_conversation(): 'some_conversation_id', MagicMock(state=MagicMock(github_token='')) ) expected = ConversationInfo( - id='some_conversation_id', + conversation_id='some_conversation_id', title='Some Conversation', last_updated_at=datetime.fromisoformat('2025-01-01T00:00:00'), status=ConversationStatus.STOPPED, @@ -103,7 +103,7 @@ async def test_update_conversation(): 'some_conversation_id', MagicMock(state=MagicMock(github_token='')) ) expected = ConversationInfo( - id='some_conversation_id', + conversation_id='some_conversation_id', title='New Title', last_updated_at=datetime.fromisoformat('2025-01-01T00:00:00'), status=ConversationStatus.STOPPED, From fb634f0d481a5ad553b0559551e7b89cdce682cc Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Thu, 2 Jan 2025 14:05:49 -0700 Subject: [PATCH 55/56] Patch is most appropriate here --- openhands/server/routes/manage_conversations.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openhands/server/routes/manage_conversations.py b/openhands/server/routes/manage_conversations.py index a3f4af42ddcd..483e64f58ed8 100644 --- a/openhands/server/routes/manage_conversations.py +++ b/openhands/server/routes/manage_conversations.py @@ -145,7 +145,7 @@ async def get_conversation( return None -@app.put('/conversations/{conversation_id}') +@app.patch('/conversations/{conversation_id}') async def update_conversation( conversation_id: str, title: str, request: Request ) -> bool: From 78ac6aa3e7298cced38186a7685d34248d9383dd Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Thu, 2 Jan 2025 14:09:23 -0700 Subject: [PATCH 56/56] Fix body params --- openhands/server/routes/manage_conversations.py | 4 ++-- tests/unit/test_conversation.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/openhands/server/routes/manage_conversations.py b/openhands/server/routes/manage_conversations.py index 483e64f58ed8..0d375229a50b 100644 --- a/openhands/server/routes/manage_conversations.py +++ b/openhands/server/routes/manage_conversations.py @@ -2,7 +2,7 @@ from datetime import datetime from typing import Callable -from fastapi import APIRouter, Request +from fastapi import APIRouter, Body, Request from fastapi.responses import JSONResponse from github import Github from pydantic import BaseModel @@ -147,7 +147,7 @@ async def get_conversation( @app.patch('/conversations/{conversation_id}') async def update_conversation( - conversation_id: str, title: str, request: Request + request: Request, conversation_id: str, title: str = Body(embed=True) ) -> bool: github_token = getattr(request.state, 'github_token', '') or '' conversation_store = await ConversationStoreImpl.get_instance(config, github_token) diff --git a/tests/unit/test_conversation.py b/tests/unit/test_conversation.py index f6e8bdd384c9..709cb4cae65f 100644 --- a/tests/unit/test_conversation.py +++ b/tests/unit/test_conversation.py @@ -95,9 +95,9 @@ async def test_get_missing_conversation(): async def test_update_conversation(): with _patch_store(): await update_conversation( + MagicMock(state=MagicMock(github_token='')), 'some_conversation_id', 'New Title', - MagicMock(state=MagicMock(github_token='')), ) conversation = await get_conversation( 'some_conversation_id', MagicMock(state=MagicMock(github_token=''))