diff --git a/backend/app/api/routes/openai_conversation.py b/backend/app/api/routes/openai_conversation.py index 5d4f8f8d..71f0c730 100644 --- a/backend/app/api/routes/openai_conversation.py +++ b/backend/app/api/routes/openai_conversation.py @@ -10,7 +10,6 @@ get_conversation_by_ancestor_id, get_conversations_by_project, get_conversations_count_by_project, - create_conversation, delete_conversation, ) from app.models import ( diff --git a/backend/app/api/routes/responses.py b/backend/app/api/routes/responses.py index 130a4839..01361803 100644 --- a/backend/app/api/routes/responses.py +++ b/backend/app/api/routes/responses.py @@ -11,7 +11,12 @@ from app.api.routes.threads import send_callback from app.crud.assistants import get_assistant_by_id from app.crud.credentials import get_provider_credential -from app.models import UserProjectOrg +from app.crud.openai_conversation import ( + create_conversation, + get_ancestor_id_from_response, + get_conversation_by_ancestor_id, +) +from app.models import UserProjectOrg, OpenAIConversationCreate from app.utils import APIResponse, mask_string from app.core.langfuse.langfuse import LangfuseTracer @@ -21,8 +26,20 @@ def handle_openai_error(e: openai.OpenAIError) -> str: """Extract error message from OpenAI error.""" - if isinstance(e.body, dict) and "message" in e.body: + # Try to get error message from different possible attributes + if hasattr(e, "body") and isinstance(e.body, dict) and "message" in e.body: return e.body["message"] + elif hasattr(e, "message"): + return e.message + elif hasattr(e, "response") and hasattr(e.response, "json"): + try: + error_data = e.response.json() + if isinstance(error_data, dict) and "error" in error_data: + error_info = error_data["error"] + if isinstance(error_info, dict) and "message" in error_info: + return error_info["message"] + except: + pass return str(e) @@ -98,6 +115,8 @@ def process_response( assistant, tracer: LangfuseTracer, project_id: int, + organization_id: int, + session: Session, ): """Process a response and send callback with results, with Langfuse tracing.""" logger.info( @@ -117,9 +136,21 @@ def process_response( ) try: + # Get the latest conversation by ancestor ID to use as previous_response_id + ancestor_id = request.response_id + latest_conversation = None + if ancestor_id: + latest_conversation = get_conversation_by_ancestor_id( + session=session, + ancestor_response_id=ancestor_id, + project_id=project_id, + ) + if latest_conversation: + ancestor_id = latest_conversation.response_id + params = { "model": assistant.model, - "previous_response_id": request.response_id, + "previous_response_id": ancestor_id, "instructions": assistant.instructions, "temperature": assistant.temperature, "input": [{"role": "user", "content": request.question}], @@ -165,6 +196,35 @@ def process_response( "error": None, }, ) + # Set ancestor_response_id using CRUD function + ancestor_response_id = ( + latest_conversation.ancestor_response_id + if latest_conversation + else get_ancestor_id_from_response( + session=session, + current_response_id=response.id, + previous_response_id=response.previous_response_id, + project_id=project_id, + ) + ) + + # Create conversation record in database + conversation_data = OpenAIConversationCreate( + response_id=response.id, + previous_response_id=response.previous_response_id, + ancestor_response_id=ancestor_response_id, + user_question=request.question, + response=response.output_text, + model=response.model, + assistant_id=request.assistant_id, + ) + + create_conversation( + session=session, + conversation=conversation_data, + project_id=project_id, + organization_id=organization_id, + ) request_dict = request.model_dump() callback_response = ResponsesAPIResponse.success_response( @@ -264,6 +324,8 @@ async def responses( assistant, tracer, project_id, + organization_id, + _session, ) logger.info( diff --git a/backend/app/crud/openai_conversation.py b/backend/app/crud/openai_conversation.py index 505e5e40..7ef127b4 100644 --- a/backend/app/crud/openai_conversation.py +++ b/backend/app/crud/openai_conversation.py @@ -1,6 +1,7 @@ import logging from typing import Optional from sqlmodel import Session, select, func + from app.models import OpenAIConversation, OpenAIConversationCreate from app.core.util import now @@ -57,6 +58,47 @@ def get_conversation_by_ancestor_id( return result +def get_ancestor_id_from_response( + session: Session, + current_response_id: str, + previous_response_id: str | None, + project_id: int, +) -> str: + """ + Set the ancestor_response_id based on previous_response_id. + + Logic: + 1. If previous_response_id is None, then ancestor_response_id = current_response_id + 2. If previous_response_id is not None, look in db for that ID + - If found, use that conversation's ancestor_id + - If not found, ancestor_response_id = previous_response_id + + Args: + session: Database session + current_response_id: The current response ID + previous_response_id: The previous response ID (can be None) + project_id: The project ID for scoping the search + + Returns: + str: The determined ancestor_response_id + """ + if previous_response_id is None: + # If previous_response_id is None, then ancestor_response_id = current_response_id + return current_response_id + + # If previous_response_id is not None, look in db for that ID + previous_conversation = get_conversation_by_response_id( + session=session, response_id=previous_response_id, project_id=project_id + ) + + if previous_conversation: + # If found, use that conversation's ancestor_id + return previous_conversation.ancestor_response_id + else: + # If not found, ancestor_response_id = previous_response_id + return previous_response_id + + def get_conversations_count_by_project( session: Session, project_id: int, diff --git a/backend/app/models/openai_conversation.py b/backend/app/models/openai_conversation.py index 93b1106a..6003c720 100644 --- a/backend/app/models/openai_conversation.py +++ b/backend/app/models/openai_conversation.py @@ -1,7 +1,7 @@ -from datetime import datetime -from typing import Optional import re +from datetime import datetime +from typing import Optional from pydantic import field_validator from sqlmodel import Field, Relationship, SQLModel diff --git a/backend/app/tests/api/routes/test_openai_conversation.py b/backend/app/tests/api/routes/test_openai_conversation.py index 55d309ed..dafc569f 100644 --- a/backend/app/tests/api/routes/test_openai_conversation.py +++ b/backend/app/tests/api/routes/test_openai_conversation.py @@ -1,8 +1,8 @@ from sqlmodel import Session from fastapi.testclient import TestClient -from app.models import APIKeyPublic from app.crud.openai_conversation import create_conversation +from app.models import APIKeyPublic from app.models import OpenAIConversationCreate from app.tests.utils.openai import generate_openai_id @@ -431,7 +431,6 @@ def test_delete_conversation_success( project_id=user_api_key.project_id, organization_id=user_api_key.organization_id, ) - conversation_id = conversation.id response = client.delete( diff --git a/backend/app/tests/api/routes/test_responses.py b/backend/app/tests/api/routes/test_responses.py index aac0a2be..483119d5 100644 --- a/backend/app/tests/api/routes/test_responses.py +++ b/backend/app/tests/api/routes/test_responses.py @@ -3,6 +3,7 @@ from fastapi import FastAPI from fastapi.testclient import TestClient from sqlmodel import select +import openai from app.api.routes.responses import router from app.models import Project @@ -15,29 +16,80 @@ @patch("app.api.routes.responses.OpenAI") @patch("app.api.routes.responses.get_provider_credential") +@patch("app.api.routes.responses.get_assistant_by_id") +@patch("app.api.routes.responses.LangfuseTracer") +@patch("app.api.routes.responses.get_ancestor_id_from_response") +@patch("app.api.routes.responses.create_conversation") +@patch("app.api.routes.responses.get_conversation_by_ancestor_id") def test_responses_endpoint_success( - mock_get_credential, mock_openai, db, user_api_key_header: dict[str, str] + mock_get_conversation_by_ancestor_id, + mock_create_conversation, + mock_get_ancestor_id_from_response, + mock_tracer_class, + mock_get_assistant, + mock_get_credential, + mock_openai, + db, + user_api_key_header: dict[str, str], ): """Test the /responses endpoint for successful response creation.""" - # Setup mock credentials - mock_get_credential.return_value = {"api_key": "test_api_key"} + + # Setup mock credentials - configure to return different values based on provider + def mock_get_credentials_by_provider(*args, **kwargs): + provider = kwargs.get("provider") + if provider == "openai": + return {"api_key": "test_api_key"} + elif provider == "langfuse": + return { + "public_key": "test_public_key", + "secret_key": "test_secret_key", + "host": "https://cloud.langfuse.com", + } + return None + + mock_get_credential.side_effect = mock_get_credentials_by_provider + + # Setup mock assistant + mock_assistant = MagicMock() + mock_assistant.model = "gpt-4o" + mock_assistant.instructions = "Test instructions" + mock_assistant.temperature = 0.1 + mock_assistant.vector_store_ids = ["vs_test"] + mock_assistant.max_num_results = 20 + + # Configure mock to return the assistant for any call + def return_mock_assistant(*args, **kwargs): + return mock_assistant + + mock_get_assistant.side_effect = return_mock_assistant # Setup mock OpenAI client mock_client = MagicMock() mock_openai.return_value = mock_client - # Setup the mock response object with real values for all used fields + # Setup the mock response object with proper response ID format mock_response = MagicMock() - mock_response.id = "mock_response_id" + mock_response.id = "resp_1234567890abcdef1234567890abcdef1234567890" mock_response.output_text = "Test output" mock_response.model = "gpt-4o" mock_response.usage.input_tokens = 10 mock_response.usage.output_tokens = 5 mock_response.usage.total_tokens = 15 mock_response.output = [] + mock_response.previous_response_id = None mock_client.responses.create.return_value = mock_response - # Get the Dalgo project ID (the assistant is created for this project) + # Setup mock tracer + mock_tracer = MagicMock() + mock_tracer_class.return_value = mock_tracer + + # Setup mock CRUD functions + mock_get_ancestor_id_from_response.return_value = ( + "resp_ancestor1234567890abcdef1234567890" + ) + mock_create_conversation.return_value = None + + # Get the Dalgo project ID dalgo_project = db.exec(select(Project).where(Project.name == "Dalgo")).first() if not dalgo_project: pytest.skip("Dalgo project not found in the database") @@ -60,7 +112,15 @@ def test_responses_endpoint_success( @patch("app.api.routes.responses.OpenAI") @patch("app.api.routes.responses.get_provider_credential") @patch("app.api.routes.responses.get_assistant_by_id") +@patch("app.api.routes.responses.LangfuseTracer") +@patch("app.api.routes.responses.get_ancestor_id_from_response") +@patch("app.api.routes.responses.create_conversation") +@patch("app.api.routes.responses.get_conversation_by_ancestor_id") def test_responses_endpoint_without_vector_store( + mock_get_conversation_by_ancestor_id, + mock_create_conversation, + mock_get_ancestor_id_from_response, + mock_tracer_class, mock_get_assistant, mock_get_credential, mock_openai, @@ -77,23 +137,35 @@ def test_responses_endpoint_without_vector_store( mock_assistant.instructions = "Test instructions" mock_assistant.temperature = 0.1 mock_assistant.vector_store_ids = [] # No vector store configured + mock_assistant.max_num_results = 20 mock_get_assistant.return_value = mock_assistant # Setup mock OpenAI client mock_client = MagicMock() mock_openai.return_value = mock_client - # Setup the mock response object + # Setup the mock response object with proper response ID format mock_response = MagicMock() - mock_response.id = "mock_response_id" + mock_response.id = "resp_1234567890abcdef1234567890abcdef1234567890" mock_response.output_text = "Test output" mock_response.model = "gpt-4" mock_response.usage.input_tokens = 10 mock_response.usage.output_tokens = 5 mock_response.usage.total_tokens = 15 - # No output attribute since there are no tool calls + mock_response.output = [] + mock_response.previous_response_id = None mock_client.responses.create.return_value = mock_response + # Setup mock tracer + mock_tracer = MagicMock() + mock_tracer_class.return_value = mock_tracer + + # Setup mock CRUD functions + mock_get_ancestor_id_from_response.return_value = ( + "resp_ancestor1234567890abcdef1234567890" + ) + mock_create_conversation.return_value = None + # Get the Glific project ID glific_project = db.exec(select(Project).where(Project.name == "Glific")).first() if not glific_project: @@ -120,3 +192,471 @@ def test_responses_endpoint_without_vector_store( temperature=mock_assistant.temperature, input=[{"role": "user", "content": "What is Glific?"}], ) + + +@patch("app.api.routes.responses.get_assistant_by_id") +def test_responses_endpoint_assistant_not_found( + mock_get_assistant, + db, + user_api_key_header, +): + """Test the /responses endpoint when assistant is not found.""" + # Setup mock assistant to return None (not found) + mock_get_assistant.return_value = None + + request_data = { + "assistant_id": "nonexistent_assistant", + "question": "What is this?", + "callback_url": "http://example.com/callback", + } + + response = client.post("/responses", json=request_data, headers=user_api_key_header) + assert response.status_code == 404 + response_json = response.json() + assert response_json["detail"] == "Assistant not found or not active" + + +@patch("app.api.routes.responses.get_provider_credential") +@patch("app.api.routes.responses.get_assistant_by_id") +def test_responses_endpoint_no_openai_credentials( + mock_get_assistant, + mock_get_credential, + db, + user_api_key_header, +): + """Test the /responses endpoint when OpenAI credentials are not configured.""" + # Setup mock assistant + mock_assistant = MagicMock() + mock_assistant.model = "gpt-4" + mock_assistant.instructions = "Test instructions" + mock_assistant.temperature = 0.1 + mock_assistant.vector_store_ids = [] + mock_get_assistant.return_value = mock_assistant + + # Setup mock credentials to return None (no credentials) + mock_get_credential.return_value = None + + request_data = { + "assistant_id": "assistant_123", + "question": "What is this?", + "callback_url": "http://example.com/callback", + } + + response = client.post("/responses", json=request_data, headers=user_api_key_header) + assert response.status_code == 200 + response_json = response.json() + assert response_json["success"] is False + assert "OpenAI API key not configured" in response_json["error"] + + +@patch("app.api.routes.responses.get_provider_credential") +@patch("app.api.routes.responses.get_assistant_by_id") +def test_responses_endpoint_missing_api_key_in_credentials( + mock_get_assistant, + mock_get_credential, + db, + user_api_key_header, +): + """Test the /responses endpoint when credentials exist but don't have api_key.""" + # Setup mock assistant + mock_assistant = MagicMock() + mock_assistant.model = "gpt-4" + mock_assistant.instructions = "Test instructions" + mock_assistant.temperature = 0.1 + mock_assistant.vector_store_ids = [] + mock_get_assistant.return_value = mock_assistant + + # Setup mock credentials without api_key + mock_get_credential.return_value = {"other_key": "value"} + + request_data = { + "assistant_id": "assistant_123", + "question": "What is this?", + "callback_url": "http://example.com/callback", + } + + response = client.post("/responses", json=request_data, headers=user_api_key_header) + assert response.status_code == 200 + response_json = response.json() + assert response_json["success"] is False + assert "OpenAI API key not configured" in response_json["error"] + + +@patch("app.api.routes.responses.OpenAI") +@patch("app.api.routes.responses.get_provider_credential") +@patch("app.api.routes.responses.get_assistant_by_id") +@patch("app.api.routes.responses.LangfuseTracer") +@patch("app.api.routes.responses.get_ancestor_id_from_response") +@patch("app.api.routes.responses.create_conversation") +@patch("app.api.routes.responses.get_conversation_by_ancestor_id") +def test_responses_endpoint_with_file_search_results( + mock_get_conversation_by_ancestor_id, + mock_create_conversation, + mock_get_ancestor_id_from_response, + mock_tracer_class, + mock_get_assistant, + mock_get_credential, + mock_openai, + db, + user_api_key_header, +): + """Test the /responses endpoint with file search results in the response.""" + # Setup mock credentials + mock_get_credential.return_value = {"api_key": "test_api_key"} + + # Setup mock assistant with vector store + mock_assistant = MagicMock() + mock_assistant.model = "gpt-4o" + mock_assistant.instructions = "Test instructions" + mock_assistant.temperature = 0.1 + mock_assistant.vector_store_ids = ["vs_test"] + mock_assistant.max_num_results = 20 + mock_get_assistant.return_value = mock_assistant + + # Setup mock OpenAI client + mock_client = MagicMock() + mock_openai.return_value = mock_client + + # Setup mock file search results + mock_hit1 = MagicMock() + mock_hit1.score = 0.95 + mock_hit1.text = "First search result" + + mock_hit2 = MagicMock() + mock_hit2.score = 0.85 + mock_hit2.text = "Second search result" + + mock_file_search_call = MagicMock() + mock_file_search_call.type = "file_search_call" + mock_file_search_call.results = [mock_hit1, mock_hit2] + + # Setup the mock response object with file search results and proper response ID format + mock_response = MagicMock() + mock_response.id = "resp_1234567890abcdef1234567890abcdef1234567890" + mock_response.output_text = "Test output with search results" + mock_response.model = "gpt-4o" + mock_response.usage.input_tokens = 10 + mock_response.usage.output_tokens = 5 + mock_response.usage.total_tokens = 15 + mock_response.output = [mock_file_search_call] + mock_response.previous_response_id = None + mock_client.responses.create.return_value = mock_response + + # Setup mock tracer + mock_tracer = MagicMock() + mock_tracer_class.return_value = mock_tracer + + # Setup mock CRUD functions + mock_get_ancestor_id_from_response.return_value = ( + "resp_ancestor1234567890abcdef1234567890" + ) + mock_create_conversation.return_value = None + + # Get the Dalgo project ID + dalgo_project = db.exec(select(Project).where(Project.name == "Dalgo")).first() + if not dalgo_project: + pytest.skip("Dalgo project not found in the database") + + request_data = { + "assistant_id": "assistant_dalgo", + "question": "What is Dalgo?", + "callback_url": "http://example.com/callback", + } + + response = client.post("/responses", json=request_data, headers=user_api_key_header) + + assert response.status_code == 200 + response_json = response.json() + assert response_json["success"] is True + assert response_json["data"]["status"] == "processing" + assert response_json["data"]["message"] == "Response creation started" + + # Verify OpenAI client was called with tools + mock_client.responses.create.assert_called_once() + call_args = mock_client.responses.create.call_args[1] + assert "tools" in call_args + assert call_args["tools"][0]["type"] == "file_search" + assert call_args["tools"][0]["vector_store_ids"] == ["vs_test"] + assert "include" in call_args + assert "file_search_call.results" in call_args["include"] + + +@patch("app.api.routes.responses.OpenAI") +@patch("app.api.routes.responses.get_provider_credential") +@patch("app.api.routes.responses.get_assistant_by_id") +@patch("app.api.routes.responses.LangfuseTracer") +@patch("app.api.routes.responses.get_ancestor_id_from_response") +@patch("app.api.routes.responses.create_conversation") +@patch("app.api.routes.responses.get_conversation_by_ancestor_id") +def test_responses_endpoint_with_ancestor_conversation_found( + mock_get_conversation_by_ancestor_id, + mock_create_conversation, + mock_get_ancestor_id_from_response, + mock_tracer_class, + mock_get_assistant, + mock_get_credential, + mock_openai, + db, + user_api_key_header: dict[str, str], +): + """Test the /responses endpoint when a conversation is found by ancestor ID.""" + # Setup mock credentials + mock_get_credential.return_value = {"api_key": "test_api_key"} + + # Setup mock assistant + mock_assistant = MagicMock() + mock_assistant.model = "gpt-4o" + mock_assistant.instructions = "Test instructions" + mock_assistant.temperature = 0.1 + mock_assistant.vector_store_ids = ["vs_test"] + mock_assistant.max_num_results = 20 + mock_get_assistant.return_value = mock_assistant + + # Setup mock OpenAI client + mock_client = MagicMock() + mock_openai.return_value = mock_client + + # Setup the mock response object + mock_response = MagicMock() + mock_response.id = "resp_1234567890abcdef1234567890abcdef1234567890" + mock_response.output_text = "Test output" + mock_response.model = "gpt-4o" + mock_response.usage.input_tokens = 10 + mock_response.usage.output_tokens = 5 + mock_response.usage.total_tokens = 15 + mock_response.output = [] + mock_response.previous_response_id = "resp_ancestor1234567890abcdef1234567890" + mock_client.responses.create.return_value = mock_response + + # Setup mock tracer + mock_tracer = MagicMock() + mock_tracer_class.return_value = mock_tracer + + # Setup mock CRUD functions + mock_get_ancestor_id_from_response.return_value = ( + "resp_ancestor1234567890abcdef1234567890" + ) + mock_create_conversation.return_value = None + + # Setup mock conversation found by ancestor ID + mock_conversation = MagicMock() + mock_conversation.response_id = "resp_latest1234567890abcdef1234567890" + mock_conversation.ancestor_response_id = "resp_ancestor1234567890abcdef1234567890" + mock_get_conversation_by_ancestor_id.return_value = mock_conversation + + # Get the Dalgo project ID + dalgo_project = db.exec(select(Project).where(Project.name == "Dalgo")).first() + if not dalgo_project: + pytest.skip("Dalgo project not found in the database") + + request_data = { + "assistant_id": "assistant_dalgo", + "question": "What is Dalgo?", + "callback_url": "http://example.com/callback", + "response_id": "resp_ancestor1234567890abcdef1234567890", + } + + response = client.post("/responses", json=request_data, headers=user_api_key_header) + + assert response.status_code == 200 + response_json = response.json() + assert response_json["success"] is True + assert response_json["data"]["status"] == "processing" + assert response_json["data"]["message"] == "Response creation started" + + # Verify get_conversation_by_ancestor_id was called with correct parameters + mock_get_conversation_by_ancestor_id.assert_called_once() + call_args = mock_get_conversation_by_ancestor_id.call_args + assert ( + call_args[1]["ancestor_response_id"] + == "resp_ancestor1234567890abcdef1234567890" + ) + assert call_args[1]["project_id"] == dalgo_project.id + + # Verify OpenAI client was called with the conversation's response_id as previous_response_id + mock_client.responses.create.assert_called_once() + call_args = mock_client.responses.create.call_args[1] + assert call_args["previous_response_id"] == "resp_latest1234567890abcdef1234567890" + + +@patch("app.api.routes.responses.OpenAI") +@patch("app.api.routes.responses.get_provider_credential") +@patch("app.api.routes.responses.get_assistant_by_id") +@patch("app.api.routes.responses.LangfuseTracer") +@patch("app.api.routes.responses.get_ancestor_id_from_response") +@patch("app.api.routes.responses.create_conversation") +@patch("app.api.routes.responses.get_conversation_by_ancestor_id") +def test_responses_endpoint_with_ancestor_conversation_not_found( + mock_get_conversation_by_ancestor_id, + mock_create_conversation, + mock_get_ancestor_id_from_response, + mock_tracer_class, + mock_get_assistant, + mock_get_credential, + mock_openai, + db, + user_api_key_header: dict[str, str], +): + """Test the /responses endpoint when no conversation is found by ancestor ID.""" + # Setup mock credentials + mock_get_credential.return_value = {"api_key": "test_api_key"} + + # Setup mock assistant + mock_assistant = MagicMock() + mock_assistant.model = "gpt-4o" + mock_assistant.instructions = "Test instructions" + mock_assistant.temperature = 0.1 + mock_assistant.vector_store_ids = ["vs_test"] + mock_assistant.max_num_results = 20 + mock_get_assistant.return_value = mock_assistant + + # Setup mock OpenAI client + mock_client = MagicMock() + mock_openai.return_value = mock_client + + # Setup the mock response object + mock_response = MagicMock() + mock_response.id = "resp_1234567890abcdef1234567890abcdef1234567890" + mock_response.output_text = "Test output" + mock_response.model = "gpt-4o" + mock_response.usage.input_tokens = 10 + mock_response.usage.output_tokens = 5 + mock_response.usage.total_tokens = 15 + mock_response.output = [] + mock_response.previous_response_id = "resp_ancestor1234567890abcdef1234567890" + mock_client.responses.create.return_value = mock_response + + # Setup mock tracer + mock_tracer = MagicMock() + mock_tracer_class.return_value = mock_tracer + + # Setup mock CRUD functions + mock_get_ancestor_id_from_response.return_value = ( + "resp_ancestor1234567890abcdef1234567890" + ) + mock_create_conversation.return_value = None + + # Setup mock conversation not found by ancestor ID + mock_get_conversation_by_ancestor_id.return_value = None + + # Get the Dalgo project ID + dalgo_project = db.exec(select(Project).where(Project.name == "Dalgo")).first() + if not dalgo_project: + pytest.skip("Dalgo project not found in the database") + + request_data = { + "assistant_id": "assistant_dalgo", + "question": "What is Dalgo?", + "callback_url": "http://example.com/callback", + "response_id": "resp_ancestor1234567890abcdef1234567890", + } + + response = client.post("/responses", json=request_data, headers=user_api_key_header) + + assert response.status_code == 200 + response_json = response.json() + assert response_json["success"] is True + assert response_json["data"]["status"] == "processing" + assert response_json["data"]["message"] == "Response creation started" + + # Verify get_conversation_by_ancestor_id was called with correct parameters + mock_get_conversation_by_ancestor_id.assert_called_once() + call_args = mock_get_conversation_by_ancestor_id.call_args + assert ( + call_args[1]["ancestor_response_id"] + == "resp_ancestor1234567890abcdef1234567890" + ) + assert call_args[1]["project_id"] == dalgo_project.id + + # Verify OpenAI client was called with the original response_id as previous_response_id + mock_client.responses.create.assert_called_once() + call_args = mock_client.responses.create.call_args[1] + assert ( + call_args["previous_response_id"] == "resp_ancestor1234567890abcdef1234567890" + ) + + +@patch("app.api.routes.responses.OpenAI") +@patch("app.api.routes.responses.get_provider_credential") +@patch("app.api.routes.responses.get_assistant_by_id") +@patch("app.api.routes.responses.LangfuseTracer") +@patch("app.api.routes.responses.get_ancestor_id_from_response") +@patch("app.api.routes.responses.create_conversation") +@patch("app.api.routes.responses.get_conversation_by_ancestor_id") +def test_responses_endpoint_without_response_id( + mock_get_conversation_by_ancestor_id, + mock_create_conversation, + mock_get_ancestor_id_from_response, + mock_tracer_class, + mock_get_assistant, + mock_get_credential, + mock_openai, + db, + user_api_key_header: dict[str, str], +): + """Test the /responses endpoint when no response_id is provided.""" + # Setup mock credentials + mock_get_credential.return_value = {"api_key": "test_api_key"} + + # Setup mock assistant + mock_assistant = MagicMock() + mock_assistant.model = "gpt-4o" + mock_assistant.instructions = "Test instructions" + mock_assistant.temperature = 0.1 + mock_assistant.vector_store_ids = ["vs_test"] + mock_assistant.max_num_results = 20 + mock_get_assistant.return_value = mock_assistant + + # Setup mock OpenAI client + mock_client = MagicMock() + mock_openai.return_value = mock_client + + # Setup the mock response object + mock_response = MagicMock() + mock_response.id = "resp_1234567890abcdef1234567890abcdef1234567890" + mock_response.output_text = "Test output" + mock_response.model = "gpt-4o" + mock_response.usage.input_tokens = 10 + mock_response.usage.output_tokens = 5 + mock_response.usage.total_tokens = 15 + mock_response.output = [] + mock_response.previous_response_id = None + mock_client.responses.create.return_value = mock_response + + # Setup mock tracer + mock_tracer = MagicMock() + mock_tracer_class.return_value = mock_tracer + + # Setup mock CRUD functions + mock_get_ancestor_id_from_response.return_value = ( + "resp_1234567890abcdef1234567890abcdef1234567890" + ) + mock_create_conversation.return_value = None + + # Get the Dalgo project ID + dalgo_project = db.exec(select(Project).where(Project.name == "Dalgo")).first() + if not dalgo_project: + pytest.skip("Dalgo project not found in the database") + + request_data = { + "assistant_id": "assistant_dalgo", + "question": "What is Dalgo?", + "callback_url": "http://example.com/callback", + # No response_id provided + } + + response = client.post("/responses", json=request_data, headers=user_api_key_header) + + assert response.status_code == 200 + response_json = response.json() + assert response_json["success"] is True + assert response_json["data"]["status"] == "processing" + assert response_json["data"]["message"] == "Response creation started" + + # Verify get_conversation_by_ancestor_id was not called since response_id is None + mock_get_conversation_by_ancestor_id.assert_not_called() + + # Verify OpenAI client was called with None as previous_response_id + mock_client.responses.create.assert_called_once() + call_args = mock_client.responses.create.call_args[1] + assert call_args["previous_response_id"] is None diff --git a/backend/app/tests/crud/test_openai_conversation.py b/backend/app/tests/crud/test_openai_conversation.py index cfb8e092..890041b0 100644 --- a/backend/app/tests/crud/test_openai_conversation.py +++ b/backend/app/tests/crud/test_openai_conversation.py @@ -1,4 +1,5 @@ import pytest +from uuid import uuid4 from sqlmodel import Session from app.crud.openai_conversation import ( @@ -6,6 +7,7 @@ get_conversation_by_response_id, get_conversation_by_ancestor_id, get_conversations_by_project, + get_ancestor_id_from_response, get_conversations_count_by_project, create_conversation, delete_conversation, @@ -84,7 +86,6 @@ def test_get_conversation_by_response_id_success(db: Session): project_id=project.id, organization_id=organization.id, ) - retrieved_conversation = get_conversation_by_response_id( session=db, response_id=conversation.response_id, @@ -324,6 +325,96 @@ def test_conversation_soft_delete_behavior(db: Session): assert conversation.id not in [c.id for c in conversations] +def test_get_ancestor_id_from_response_no_previous_response(db: Session): + """Test get_ancestor_id_from_response when previous_response_id is None.""" + project = get_project(db) + current_response_id = generate_openai_id("resp_", 40) + + ancestor_id = get_ancestor_id_from_response( + session=db, + current_response_id=current_response_id, + previous_response_id=None, + project_id=project.id, + ) + + assert ancestor_id == current_response_id + + +def test_get_ancestor_id_from_response_previous_not_found(db: Session): + """Test get_ancestor_id_from_response when previous_response_id is not found in DB.""" + project = get_project(db) + current_response_id = generate_openai_id("resp_", 40) + previous_response_id = generate_openai_id("resp_", 40) + + ancestor_id = get_ancestor_id_from_response( + session=db, + current_response_id=current_response_id, + previous_response_id=previous_response_id, + project_id=project.id, + ) + + # When previous_response_id is not found, should return previous_response_id + assert ancestor_id == previous_response_id + + +def test_get_ancestor_id_from_response_previous_found_with_ancestor(db: Session): + """Test get_ancestor_id_from_response when previous_response_id is found and has an ancestor.""" + project = get_project(db) + organization = get_organization(db) + + # Create a conversation chain: ancestor -> previous -> current + ancestor_response_id = generate_openai_id("resp_", 40) + + # Create the ancestor conversation + ancestor_conversation_data = OpenAIConversationCreate( + response_id=ancestor_response_id, + ancestor_response_id=ancestor_response_id, # Self-referencing + previous_response_id=None, + user_question="Original question", + response="Original response", + model="gpt-4o", + assistant_id=generate_openai_id("asst_", 20), + ) + + ancestor_conversation = create_conversation( + session=db, + conversation=ancestor_conversation_data, + project_id=project.id, + organization_id=organization.id, + ) + + # Create the previous conversation + previous_response_id = generate_openai_id("resp_", 40) + previous_conversation_data = OpenAIConversationCreate( + response_id=previous_response_id, + ancestor_response_id=ancestor_response_id, + previous_response_id=ancestor_response_id, + user_question="Previous question", + response="Previous response", + model="gpt-4o", + assistant_id=generate_openai_id("asst_", 20), + ) + + create_conversation( + session=db, + conversation=previous_conversation_data, + project_id=project.id, + organization_id=organization.id, + ) + + # Test the current conversation + current_response_id = generate_openai_id("resp_", 40) + ancestor_id = get_ancestor_id_from_response( + session=db, + current_response_id=current_response_id, + previous_response_id=previous_response_id, + project_id=project.id, + ) + + # Should return the ancestor_response_id from the previous conversation + assert ancestor_id == ancestor_response_id + + def test_get_conversations_count_by_project_success(db: Session): """Test successful conversation count retrieval by project.""" project = get_project(db) @@ -362,8 +453,202 @@ def test_get_conversations_count_by_project_success(db: Session): assert updated_count == initial_count + 3 -def test_get_conversations_count_by_project_excludes_deleted(db: Session): - """Test that deleted conversations are not counted.""" +def test_get_ancestor_id_from_response_previous_found_without_ancestor(db: Session): + """Test get_ancestor_id_from_response when previous_response_id is found but has no ancestor.""" + project = get_project(db) + organization = get_organization(db) + + # Create a previous conversation that is self-referencing + previous_response_id = generate_openai_id("resp_", 40) + previous_conversation_data = OpenAIConversationCreate( + response_id=previous_response_id, + ancestor_response_id=previous_response_id, # Self-referencing for root + previous_response_id=None, + user_question="Previous question", + response="Previous response", + model="gpt-4o", + assistant_id=generate_openai_id("asst_", 20), + ) + + create_conversation( + session=db, + conversation=previous_conversation_data, + project_id=project.id, + organization_id=organization.id, + ) + + # Test the current conversation + current_response_id = generate_openai_id("resp_", 40) + ancestor_id = get_ancestor_id_from_response( + session=db, + current_response_id=current_response_id, + previous_response_id=previous_response_id, + project_id=project.id, + ) + + # When previous conversation is root (self-referencing), should return the previous_response_id + assert ancestor_id == previous_response_id + + +def test_get_ancestor_id_from_response_different_project(db: Session): + """Test get_ancestor_id_from_response respects project scoping.""" + project1 = get_project(db) + organization = get_organization(db) + + # Create a second project with a different name + from app.models import Project + + project2 = Project( + name=f"test_project_{uuid4()}", + description="Test project for scoping", + is_active=True, + organization_id=organization.id, + ) + db.add(project2) + db.commit() + db.refresh(project2) + + # Create a conversation in project1 + previous_response_id = generate_openai_id("resp_", 40) + previous_conversation_data = OpenAIConversationCreate( + response_id=previous_response_id, + ancestor_response_id=generate_openai_id("resp_", 40), + previous_response_id=None, + user_question="Previous question", + response="Previous response", + model="gpt-4o", + assistant_id=generate_openai_id("asst_", 20), + ) + + create_conversation( + session=db, + conversation=previous_conversation_data, + project_id=project1.id, + organization_id=organization.id, + ) + + # Test looking for it in project2 (should not find it) + current_response_id = generate_openai_id("resp_", 40) + ancestor_id = get_ancestor_id_from_response( + session=db, + current_response_id=current_response_id, + previous_response_id=previous_response_id, + project_id=project2.id, + ) + + # Should return previous_response_id since it's not found in project2 + assert ancestor_id == previous_response_id + + +def test_get_ancestor_id_from_response_complex_chain(db: Session): + """Test get_ancestor_id_from_response with a complex conversation chain.""" + project = get_project(db) + organization = get_organization(db) + + # Create a complex chain: A -> B -> C -> D + # A is the root ancestor + response_a = generate_openai_id("resp_", 40) + conversation_a_data = OpenAIConversationCreate( + response_id=response_a, + ancestor_response_id=response_a, # Self-referencing + previous_response_id=None, + user_question="Question A", + response="Response A", + model="gpt-4o", + assistant_id=generate_openai_id("asst_", 20), + ) + + create_conversation( + session=db, + conversation=conversation_a_data, + project_id=project.id, + organization_id=organization.id, + ) + + # B references A + response_b = generate_openai_id("resp_", 40) + conversation_b_data = OpenAIConversationCreate( + response_id=response_b, + ancestor_response_id=response_a, + previous_response_id=response_a, + user_question="Question B", + response="Response B", + model="gpt-4o", + assistant_id=generate_openai_id("asst_", 20), + ) + + create_conversation( + session=db, + conversation=conversation_b_data, + project_id=project.id, + organization_id=organization.id, + ) + + # C references B + response_c = generate_openai_id("resp_", 40) + conversation_c_data = OpenAIConversationCreate( + response_id=response_c, + ancestor_response_id=response_a, # Should inherit from B + previous_response_id=response_b, + user_question="Question C", + response="Response C", + model="gpt-4o", + assistant_id=generate_openai_id("asst_", 20), + ) + + create_conversation( + session=db, + conversation=conversation_c_data, + project_id=project.id, + organization_id=organization.id, + ) + + # Test D referencing C + response_d = generate_openai_id("resp_", 40) + ancestor_id = get_ancestor_id_from_response( + session=db, + current_response_id=response_d, + previous_response_id=response_c, + project_id=project.id, + ) + + # Should return response_a (the root ancestor) + assert ancestor_id == response_a + + +def test_create_conversation_success(db: Session): + """Test successful conversation creation.""" + project = get_project(db) + organization = get_organization(db) + + conversation_data = OpenAIConversationCreate( + response_id=generate_openai_id("resp_", 40), + ancestor_response_id=generate_openai_id("resp_", 40), + previous_response_id=None, + user_question="Test question", + response="Test response", + model="gpt-4o", + assistant_id=generate_openai_id("asst_", 20), + ) + + conversation = create_conversation( + session=db, + conversation=conversation_data, + project_id=project.id, + organization_id=organization.id, + ) + + assert conversation is not None + assert conversation.response_id == conversation_data.response_id + assert conversation.user_question == conversation_data.user_question + assert conversation.response == conversation_data.response + assert conversation.model == conversation_data.model + assert conversation.project_id == project.id + assert conversation.organization_id == organization.id + + +def test_delete_conversation_excludes_from_count(db: Session): + """Test that deleted conversations are excluded from count.""" project = get_project(db) organization = get_organization(db) @@ -488,7 +773,46 @@ def test_response_id_validation_pattern(db: Session): organization_id=organization.id, ) assert conversation is not None - assert conversation.response_id == valid_response_id + assert conversation.response_id == conversation_data.response_id + assert conversation.user_question == conversation_data.user_question + assert conversation.response == conversation_data.response + assert conversation.model == conversation_data.model + assert conversation.assistant_id == conversation_data.assistant_id + assert conversation.project_id == project.id + assert conversation.organization_id == organization.id + assert conversation.is_deleted is False + assert conversation.deleted_at is None + + +def test_create_conversation_with_ancestor(db: Session): + """Test conversation creation with ancestor and previous response IDs.""" + project = get_project(db) + organization = get_organization(db) + + ancestor_response_id = generate_openai_id("resp_", 40) + previous_response_id = generate_openai_id("resp_", 40) + + conversation_data = OpenAIConversationCreate( + response_id=generate_openai_id("resp_", 40), + ancestor_response_id=ancestor_response_id, + previous_response_id=previous_response_id, + user_question="Follow-up question", + response="Follow-up response", + model="gpt-4o", + assistant_id=generate_openai_id("asst_", 20), + ) + + conversation = create_conversation( + session=db, + conversation=conversation_data, + project_id=project.id, + organization_id=organization.id, + ) + + assert conversation is not None + assert conversation.ancestor_response_id == ancestor_response_id + assert conversation.previous_response_id == previous_response_id + assert conversation.response_id == conversation_data.response_id # Test invalid response ID (too short) invalid_response_id = "resp_123"