From 095947dd88ec1424b64a7ed92afda43c204a49c3 Mon Sep 17 00:00:00 2001 From: Stephanie Date: Wed, 13 Aug 2025 13:38:42 -0400 Subject: [PATCH 1/2] fix conversation delete Signed-off-by: Stephanie --- src/app/endpoints/conversations.py | 34 +++++++++++++++++++++++++----- src/utils/endpoints.py | 9 ++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/src/app/endpoints/conversations.py b/src/app/endpoints/conversations.py index 38d25b1b9..40a78c2e7 100644 --- a/src/app/endpoints/conversations.py +++ b/src/app/endpoints/conversations.py @@ -248,6 +248,15 @@ async def get_conversation_endpoint_handler( client = AsyncLlamaStackClientHolder().get_client() agent_sessions = (await client.agents.session.list(agent_id=agent_id)).data + if not agent_sessions: + logger.error("No sessions found for conversation %s", conversation_id) + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail={ + "response": "Conversation not found", + "cause": f"Conversation {conversation_id} could not be deleted: no sessions found", + }, + ) session_id = str(agent_sessions[0].get("session_id")) session_response = await client.agents.session.retrieve( @@ -283,6 +292,8 @@ async def get_conversation_endpoint_handler( "cause": f"Conversation {conversation_id} could not be retrieved: {str(e)}", }, ) from e + except HTTPException: + raise except Exception as e: # Handle case where session doesn't exist or other errors logger.exception("Error retrieving conversation %s: %s", conversation_id, e) @@ -339,11 +350,22 @@ async def delete_conversation_endpoint_handler( try: # Get Llama Stack client client = AsyncLlamaStackClientHolder().get_client() - # Delete session using the conversation_id as session_id - # In this implementation, conversation_id and session_id are the same - await client.agents.session.delete( - agent_id=agent_id, session_id=conversation_id - ) + + agent_sessions = (await client.agents.session.list(agent_id=agent_id)).data + + if not agent_sessions: + logger.error("No sessions found for conversation %s", conversation_id) + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail={ + "response": "Conversation not found", + "cause": f"Conversation {conversation_id} could not be deleted: no sessions found", + }, + ) + + session_id = str(agent_sessions[0].get("session_id")) + + await client.agents.session.delete(agent_id=agent_id, session_id=session_id) logger.info("Successfully deleted conversation %s", conversation_id) @@ -371,6 +393,8 @@ async def delete_conversation_endpoint_handler( "cause": f"Conversation {conversation_id} could not be deleted: {str(e)}", }, ) from e + except HTTPException: + raise except Exception as e: # Handle case where session doesn't exist or other errors logger.exception("Error deleting conversation %s: %s", conversation_id, e) diff --git a/src/utils/endpoints.py b/src/utils/endpoints.py index 7e1c98bd5..032c2b742 100644 --- a/src/utils/endpoints.py +++ b/src/utils/endpoints.py @@ -109,6 +109,15 @@ async def get_agent( await client.agents.delete(agent_id=orphan_agent_id) sessions_response = await client.agents.session.list(agent_id=conversation_id) logger.info("session response: %s", sessions_response) + if not sessions_response or not sessions_response.data: + logger.error("No sessions found for conversation %s", conversation_id) + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail={ + "response": "Conversation not found", + "cause": f"Conversation {conversation_id} could not be retrieved.", + }, + ) session_id = str(sessions_response.data[0]["session_id"]) else: conversation_id = agent.agent_id From 19fd555a6773ee5ec0e7687afc360ccd1a912d9a Mon Sep 17 00:00:00 2001 From: Stephanie Date: Thu, 14 Aug 2025 12:41:39 -0400 Subject: [PATCH 2/2] address review comments and resolve test failures Signed-off-by: Stephanie --- src/app/endpoints/conversations.py | 15 +++++++-------- src/utils/endpoints.py | 7 ++++--- tests/unit/app/endpoints/test_conversations.py | 4 ++++ 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/app/endpoints/conversations.py b/src/app/endpoints/conversations.py index 40a78c2e7..bbb7825d1 100644 --- a/src/app/endpoints/conversations.py +++ b/src/app/endpoints/conversations.py @@ -254,7 +254,7 @@ async def get_conversation_endpoint_handler( status_code=status.HTTP_404_NOT_FOUND, detail={ "response": "Conversation not found", - "cause": f"Conversation {conversation_id} could not be deleted: no sessions found", + "cause": f"Conversation {conversation_id} could not be retrieved.", }, ) session_id = str(agent_sessions[0].get("session_id")) @@ -354,13 +354,12 @@ async def delete_conversation_endpoint_handler( agent_sessions = (await client.agents.session.list(agent_id=agent_id)).data if not agent_sessions: - logger.error("No sessions found for conversation %s", conversation_id) - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, - detail={ - "response": "Conversation not found", - "cause": f"Conversation {conversation_id} could not be deleted: no sessions found", - }, + # If no sessions are found, do not raise an error, just return a success response + logger.info("No sessions found for conversation %s", conversation_id) + return ConversationDeleteResponse( + conversation_id=conversation_id, + success=True, + response="Conversation deleted successfully", ) session_id = str(agent_sessions[0].get("session_id")) diff --git a/src/utils/endpoints.py b/src/utils/endpoints.py index 032c2b742..993dc4e83 100644 --- a/src/utils/endpoints.py +++ b/src/utils/endpoints.py @@ -109,7 +109,9 @@ async def get_agent( await client.agents.delete(agent_id=orphan_agent_id) sessions_response = await client.agents.session.list(agent_id=conversation_id) logger.info("session response: %s", sessions_response) - if not sessions_response or not sessions_response.data: + try: + session_id = str(sessions_response.data[0]["session_id"]) + except IndexError as e: logger.error("No sessions found for conversation %s", conversation_id) raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, @@ -117,8 +119,7 @@ async def get_agent( "response": "Conversation not found", "cause": f"Conversation {conversation_id} could not be retrieved.", }, - ) - session_id = str(sessions_response.data[0]["session_id"]) + ) from e else: conversation_id = agent.agent_id session_id = await agent.create_session(get_suid()) diff --git a/tests/unit/app/endpoints/test_conversations.py b/tests/unit/app/endpoints/test_conversations.py index d07416b78..aed6fdc5b 100644 --- a/tests/unit/app/endpoints/test_conversations.py +++ b/tests/unit/app/endpoints/test_conversations.py @@ -490,6 +490,10 @@ async def test_successful_conversation_deletion(self, mocker, setup_configuratio # Mock AsyncLlamaStackClientHolder mock_client = mocker.AsyncMock() + # Ensure the endpoint sees an existing session so it proceeds to delete + mock_client.agents.session.list.return_value = mocker.Mock( + data=[{"session_id": VALID_CONVERSATION_ID}] + ) mock_client.agents.session.delete.return_value = None # Successful deletion mock_client_holder = mocker.patch( "app.endpoints.conversations.AsyncLlamaStackClientHolder"