diff --git a/backend/app/alembic/versions/e8ee93526b37_add_is_deleted_column_in_assistant_table.py b/backend/app/alembic/versions/e8ee93526b37_add_is_deleted_column_in_assistant_table.py new file mode 100644 index 00000000..5cea5342 --- /dev/null +++ b/backend/app/alembic/versions/e8ee93526b37_add_is_deleted_column_in_assistant_table.py @@ -0,0 +1,30 @@ +"""Add is_deleted column in assistant table + +Revision ID: e8ee93526b37 +Revises: 4aa1f48c6321 +Create Date: 2025-07-21 12:40:03.791321 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = "e8ee93526b37" +down_revision = "4aa1f48c6321" +branch_labels = None +depends_on = None + + +def upgrade(): + op.add_column( + "openai_assistant", sa.Column("is_deleted", sa.Boolean(), nullable=False) + ) + op.add_column( + "openai_assistant", sa.Column("deleted_at", sa.DateTime(), nullable=True) + ) + + +def downgrade(): + op.drop_column("openai_assistant", "deleted_at") + op.drop_column("openai_assistant", "is_deleted") diff --git a/backend/app/api/routes/assistants.py b/backend/app/api/routes/assistants.py index 35f66146..3ed327f2 100644 --- a/backend/app/api/routes/assistants.py +++ b/backend/app/api/routes/assistants.py @@ -1,14 +1,19 @@ from typing import Annotated -from fastapi import APIRouter, Depends, Path +from fastapi import APIRouter, Depends, Path, HTTPException, Query from sqlmodel import Session from app.api.deps import get_db, get_current_user_org_project from app.crud import ( fetch_assistant_from_openai, sync_assistant, + create_assistant, + update_assistant, + get_assistant_by_id, + get_assistants_by_project, + delete_assistant, ) -from app.models import UserProjectOrg +from app.models import UserProjectOrg, AssistantCreate, AssistantUpdate, Assistant from app.utils import APIResponse, get_openai_client router = APIRouter(prefix="/assistant", tags=["Assistants"]) @@ -16,7 +21,7 @@ @router.post( "/{assistant_id}/ingest", - response_model=APIResponse, + response_model=APIResponse[Assistant], status_code=201, ) def ingest_assistant_route( @@ -41,3 +46,109 @@ def ingest_assistant_route( ) return APIResponse.success_response(assistant) + + +@router.post("/", response_model=APIResponse[Assistant], status_code=201) +def create_assistant_route( + assistant_in: AssistantCreate, + session: Session = Depends(get_db), + current_user: UserProjectOrg = Depends(get_current_user_org_project), +): + """ + Create a new assistant in the local DB, checking that vector store IDs exist in OpenAI first. + """ + client = get_openai_client( + session, current_user.organization_id, current_user.project_id + ) + assistant = create_assistant( + session=session, + openai_client=client, + assistant=assistant_in, + project_id=current_user.project_id, + organization_id=current_user.organization_id, + ) + return APIResponse.success_response(assistant) + + +@router.patch("/{assistant_id}", response_model=APIResponse[Assistant]) +def update_assistant_route( + assistant_id: Annotated[str, Path(description="Assistant ID to update")], + assistant_update: AssistantUpdate, + session: Session = Depends(get_db), + current_user: UserProjectOrg = Depends(get_current_user_org_project), +): + """ + Update an existing assistant with provided fields. Supports replacing, adding, or removing vector store IDs. + """ + client = get_openai_client( + session, current_user.organization_id, current_user.project_id + ) + updated_assistant = update_assistant( + session=session, + assistant_id=assistant_id, + openai_client=client, + project_id=current_user.project_id, + assistant_update=assistant_update, + ) + return APIResponse.success_response(updated_assistant) + + +@router.get( + "/{assistant_id}", + response_model=APIResponse[Assistant], + summary="Get a single assistant by its ID", +) +def get_assistant_route( + assistant_id: str = Path(..., description="The assistant_id to fetch"), + session: Session = Depends(get_db), + current_user: UserProjectOrg = Depends(get_current_user_org_project), +): + """ + Fetch a single assistant by its assistant_id. + """ + assistant = get_assistant_by_id(session, assistant_id, current_user.project_id) + if not assistant: + raise HTTPException( + status_code=404, detail=f"Assistant with ID {assistant_id} not found." + ) + return APIResponse.success_response(assistant) + + +@router.get( + "/", + response_model=APIResponse[list[Assistant]], + summary="List all assistants in the current project", +) +def list_assistants_route( + session: Session = Depends(get_db), + current_user: UserProjectOrg = Depends(get_current_user_org_project), + skip: int = Query(0, ge=0, description="How many items to skip"), + limit: int = Query(100, ge=1, le=100, description="Maximum items to return"), +): + """ + List all assistants in the current project and organization. + """ + + assistants = get_assistants_by_project( + session=session, project_id=current_user.project_id, skip=skip, limit=limit + ) + return APIResponse.success_response(assistants) + + +@router.delete("/{assistant_id}", response_model=APIResponse) +def delete_assistant_route( + assistant_id: Annotated[str, Path(description="Assistant ID to delete")], + session: Session = Depends(get_db), + current_user: UserProjectOrg = Depends(get_current_user_org_project), +): + """ + Soft delete an assistant by marking it as deleted. + """ + delete_assistant( + session=session, + assistant_id=assistant_id, + project_id=current_user.project_id, + ) + return APIResponse.success_response( + data={"message": "Assistant deleted successfully."} + ) diff --git a/backend/app/api/routes/responses.py b/backend/app/api/routes/responses.py index 50121fe0..130a4839 100644 --- a/backend/app/api/routes/responses.py +++ b/backend/app/api/routes/responses.py @@ -220,7 +220,7 @@ async def responses( f"Processing response request for assistant_id={mask_string(request.assistant_id)}, project_id={project_id}, organization_id={organization_id}" ) - assistant = get_assistant_by_id(_session, request.assistant_id, organization_id) + assistant = get_assistant_by_id(_session, request.assistant_id, project_id) if not assistant: logger.warning( f"Assistant not found: assistant_id={mask_string(request.assistant_id)}, project_id={project_id}, organization_id={organization_id}", diff --git a/backend/app/crud/__init__.py b/backend/app/crud/__init__.py index 1746b32c..49b09f56 100644 --- a/backend/app/crud/__init__.py +++ b/backend/app/crud/__init__.py @@ -49,4 +49,8 @@ get_assistant_by_id, fetch_assistant_from_openai, sync_assistant, + create_assistant, + update_assistant, + get_assistants_by_project, + delete_assistant, ) diff --git a/backend/app/crud/assistants.py b/backend/app/crud/assistants.py index 3f040508..6a8e8e5b 100644 --- a/backend/app/crud/assistants.py +++ b/backend/app/crud/assistants.py @@ -1,6 +1,7 @@ import logging from typing import Optional +import uuid import openai from fastapi import HTTPException @@ -8,25 +9,49 @@ from openai.types.beta import Assistant as OpenAIAssistant from sqlmodel import Session, and_, select -from app.models import Assistant +from app.models import Assistant, AssistantCreate, AssistantUpdate from app.utils import mask_string +from app.core.util import now logger = logging.getLogger(__name__) def get_assistant_by_id( - session: Session, assistant_id: str, organization_id: int + session: Session, assistant_id: str, project_id: int ) -> Optional[Assistant]: """Get an assistant by its OpenAI assistant ID and organization ID.""" statement = select(Assistant).where( and_( Assistant.assistant_id == assistant_id, - Assistant.organization_id == organization_id, + Assistant.project_id == project_id, + Assistant.is_deleted == False, ) ) return session.exec(statement).first() +def get_assistants_by_project( + session: Session, + project_id: int, + skip: int = 0, + limit: int = 100, +) -> list[Assistant]: + """ + Return all assistants for a given project and organization, with optional pagination. + """ + statement = ( + select(Assistant) + .where( + Assistant.project_id == project_id, + Assistant.is_deleted == False, + ) + .offset(skip) + .limit(limit) + ) + results = session.exec(statement).all() + return results + + def fetch_assistant_from_openai(assistant_id: str, client: OpenAI) -> OpenAIAssistant: """ Fetch an assistant from OpenAI. @@ -48,6 +73,29 @@ def fetch_assistant_from_openai(assistant_id: str, client: OpenAI) -> OpenAIAssi raise HTTPException(status_code=502, detail=f"OpenAI API error: {e}") +def verify_vector_store_ids_exist( + openai_client: OpenAI, vector_store_ids: list[str] +) -> None: + """ + Raises HTTPException if any of the vector_store_ids do not exist in OpenAI. + """ + for vector_store_id in vector_store_ids: + try: + openai_client.vector_stores.retrieve(vector_store_id) + except openai.NotFoundError: + logger.error(f"Vector store ID {vector_store_id} not found in OpenAI.") + raise HTTPException( + status_code=400, + detail=f"Vector store ID {vector_store_id} not found in OpenAI.", + ) + except openai.OpenAIError as e: + logger.error(f"Failed to verify vector store ID {vector_store_id}: {e}") + raise HTTPException( + status_code=502, + detail=f"Error verifying vector store ID {vector_store_id}: {str(e)}", + ) + + def sync_assistant( session: Session, organization_id: int, @@ -59,10 +107,10 @@ def sync_assistant( """ assistant_id = openai_assistant.id - existing_assistant = get_assistant_by_id(session, assistant_id, organization_id) + existing_assistant = get_assistant_by_id(session, assistant_id, project_id) if existing_assistant: - logger.info( - f"[sync_assistant] Assistant with ID {mask_string(assistant_id)} already exists in the database." + logger.warning( + f"[sync_assistant] Assistant with ID {mask_string(assistant_id)} already exists in the database. | project_id: {project_id}" ) raise HTTPException( status_code=409, @@ -70,6 +118,9 @@ def sync_assistant( ) if not openai_assistant.instructions: + logger.warning( + f"[sync_assistant] OpenAI assistant {mask_string(assistant_id)} has no instructions. | project_id: {project_id}" + ) raise HTTPException( status_code=400, detail="Assistant has no instruction.", @@ -111,3 +162,111 @@ def sync_assistant( f"[sync_assistant] Successfully ingested assistant with ID {mask_string(assistant_id)}." ) return db_assistant + + +def create_assistant( + session: Session, + openai_client: OpenAI, + assistant: AssistantCreate, + project_id: int, + organization_id: int, +) -> Assistant: + verify_vector_store_ids_exist(openai_client, assistant.vector_store_ids) + + assistant = Assistant( + assistant_id=str(uuid.uuid4()), + **assistant.model_dump(exclude_unset=True), + project_id=project_id, + organization_id=organization_id, + ) + session.add(assistant) + session.commit() + session.refresh(assistant) + logger.info( + f"[create_assistant] Assistant created successfully. | project_id: {project_id}, assistant_id: {mask_string(assistant.assistant_id)}" + ) + return assistant + + +def update_assistant( + session: Session, + openai_client: OpenAI, + assistant_id: str, + project_id: int, + assistant_update: AssistantUpdate, +) -> Assistant: + existing_assistant = get_assistant_by_id(session, assistant_id, project_id) + if not existing_assistant: + logger.error( + f"[update_assistant] Assistant {mask_string(assistant_id)} not found | project_id: {project_id}" + ) + raise HTTPException(status_code=404, detail="Assistant not found.") + + # Update non-vector_store_ids fields, if present + update_fields = assistant_update.model_dump( + exclude_unset=True, exclude={"vector_store_ids_add", "vector_store_ids_remove"} + ) + for field, value in update_fields.items(): + setattr(existing_assistant, field, value) + + current_vector_stores: set[str] = set(existing_assistant.vector_store_ids or []) + + # Validate for conflicting add/remove operations + add_ids = set(assistant_update.vector_store_ids_add or []) + remove_ids = set(assistant_update.vector_store_ids_remove or []) + if conflicting_ids := add_ids & remove_ids: + logger.error( + f"[update_assistant] Conflicting vector store IDs in add/remove: {conflicting_ids} | project_id: {project_id}" + ) + raise HTTPException( + status_code=400, + detail=f"Conflicting vector store IDs in add/remove: {conflicting_ids}.", + ) + + # Add new vector store IDs + if add_ids: + verify_vector_store_ids_exist(openai_client, list(add_ids)) + current_vector_stores.update(add_ids) + + # Remove vector store IDs + if remove_ids: + current_vector_stores.difference_update(remove_ids) + + # Update assistant's vector store IDs + existing_assistant.vector_store_ids = list(current_vector_stores) + existing_assistant.updated_at = now() + session.add(existing_assistant) + session.commit() + session.refresh(existing_assistant) + + logger.info( + f"[update_assistant] Assistant {mask_string(assistant_id)} updated successfully. | project_id: {project_id}" + ) + return existing_assistant + + +def delete_assistant( + session: Session, + assistant_id: str, + project_id: int, +) -> Assistant: + """ + Soft delete an assistant by marking it as deleted. + """ + existing_assistant = get_assistant_by_id(session, assistant_id, project_id) + if not existing_assistant: + logger.warning( + f"[delete_assistant] Assistant {mask_string(assistant_id)} not found | project_id: {project_id}" + ) + raise HTTPException(status_code=404, detail="Assistant not found.") + + existing_assistant.is_deleted = True + existing_assistant.deleted_at = now() + session.add(existing_assistant) + session.commit() + session.refresh(existing_assistant) + + logger.info( + f"[delete_assistant] Assistant {mask_string(assistant_id)} soft deleted successfully. | project_id: {project_id}" + ) + return existing_assistant diff --git a/backend/app/models/__init__.py b/backend/app/models/__init__.py index 04693637..2c4c87e0 100644 --- a/backend/app/models/__init__.py +++ b/backend/app/models/__init__.py @@ -54,4 +54,4 @@ from .threads import OpenAI_Thread, OpenAIThreadBase, OpenAIThreadCreate -from .assistants import Assistant, AssistantBase +from .assistants import Assistant, AssistantBase, AssistantCreate, AssistantUpdate diff --git a/backend/app/models/assistants.py b/backend/app/models/assistants.py index 8546a074..b97e25b1 100644 --- a/backend/app/models/assistants.py +++ b/backend/app/models/assistants.py @@ -1,8 +1,9 @@ from datetime import datetime -from typing import Optional, List -from sqlmodel import Field, Relationship, SQLModel +from typing import List, Optional + from sqlalchemy import Column, String, Text from sqlalchemy.dialects.postgresql import ARRAY +from sqlmodel import Field, Relationship, SQLModel from app.core.util import now @@ -31,7 +32,60 @@ class Assistant(AssistantBase, table=True): id: int = Field(default=None, primary_key=True) inserted_at: datetime = Field(default_factory=now, nullable=False) updated_at: datetime = Field(default_factory=now, nullable=False) + is_deleted: bool = Field(default=False, nullable=False) + deleted_at: Optional[datetime] = Field(default=None, nullable=True) # Relationships project: "Project" = Relationship(back_populates="assistants") organization: "Organization" = Relationship(back_populates="assistants") + + +class AssistantCreate(SQLModel): + name: str = Field(description="Name of the assistant", min_length=3, max_length=50) + instructions: str = Field( + description="Instructions for the assistant", min_length=10 + ) + model: str = Field( + default="gpt-4o", + description="Model name for the assistant", + min_length=1, + max_length=50, + ) + vector_store_ids: list[str] = Field( + default_factory=list, + description="List of Vector Store IDs that exist in OpenAI.", + ) + temperature: float = Field( + default=0.1, gt=0, le=2, description="Sampling temperature between 0 and 2" + ) + max_num_results: int = Field( + default=20, + ge=1, + le=100, + description="Maximum number of results (must be between 1 and 100)", + ) + + +class AssistantUpdate(SQLModel): + name: str | None = Field( + default=None, description="Name of the assistant", min_length=3, max_length=50 + ) + instructions: str | None = Field( + default=None, + description="Instructions for the assistant", + min_length=10, + ) + model: str | None = Field( + default=None, description="Name of the model", min_length=1, max_length=50 + ) + vector_store_ids_add: list[str] | None = None + vector_store_ids_remove: list[str] | None = None + temperature: float | None = Field( + default=None, gt=0, le=2, description="Sampling temperature between 0 and 2" + ) + max_num_results: int | None = Field( + default=None, + ge=1, + le=100, + description="Maximum number of results (must be between 1 and 100)", + ) diff --git a/backend/app/tests/api/routes/test_assistants.py b/backend/app/tests/api/routes/test_assistants.py index b918c621..8755889d 100644 --- a/backend/app/tests/api/routes/test_assistants.py +++ b/backend/app/tests/api/routes/test_assistants.py @@ -1,7 +1,29 @@ import pytest +from uuid import uuid4 +from sqlmodel import Session +from fastapi import HTTPException from fastapi.testclient import TestClient from unittest.mock import patch +from app.crud.api_key import get_api_keys_by_project from app.tests.utils.openai import mock_openai_assistant +from app.tests.utils.utils import get_assistant + + +@pytest.fixture +def assistant_create_payload(): + return { + "name": "Test Assistant", + "instructions": "This is a test instruction.", + "model": "gpt-4o", + "vector_store_ids": ["vs_test_1", "vs_test_2"], + "temperature": 0.5, + "max_num_results": 10, + } + + +@pytest.fixture +def assistant_id(): + return str(uuid4()) @patch("app.api.routes.assistants.fetch_assistant_from_openai") @@ -24,3 +46,274 @@ def test_ingest_assistant_success( response_json = response.json() assert response_json["success"] is True assert response_json["data"]["assistant_id"] == mock_assistant.id + + +@patch("app.crud.assistants.verify_vector_store_ids_exist") +def test_create_assistant_success( + mock_verify_vector_ids, + client: TestClient, + assistant_create_payload: dict, + normal_user_api_key_headers: dict, +): + """Test successful assistant creation with OpenAI vector store ID verification.""" + + mock_verify_vector_ids.return_value = None + + response = client.post( + "/api/v1/assistant", + json=assistant_create_payload, + headers=normal_user_api_key_headers, + ) + + assert response.status_code == 201 + response_data = response.json() + assert response_data["success"] is True + assert response_data["data"]["name"] == assistant_create_payload["name"] + assert ( + response_data["data"]["instructions"] + == assistant_create_payload["instructions"] + ) + assert response_data["data"]["model"] == assistant_create_payload["model"] + assert ( + response_data["data"]["vector_store_ids"] + == assistant_create_payload["vector_store_ids"] + ) + assert ( + response_data["data"]["temperature"] == assistant_create_payload["temperature"] + ) + assert ( + response_data["data"]["max_num_results"] + == assistant_create_payload["max_num_results"] + ) + + +@patch("app.crud.assistants.verify_vector_store_ids_exist") +def test_create_assistant_invalid_vector_store( + mock_verify_vector_ids, + client: TestClient, + assistant_create_payload: dict, + normal_user_api_key_headers: dict, +): + """Test failure when one or more vector store IDs are invalid.""" + + mock_verify_vector_ids.side_effect = HTTPException( + status_code=400, detail="Vector store ID vs_test_999 not found in OpenAI." + ) + + payload = assistant_create_payload.copy() + payload["vector_store_ids"] = ["vs_test_999"] + + response = client.post( + "/api/v1/assistant", + json=payload, + headers=normal_user_api_key_headers, + ) + + assert response.status_code == 400 + response_data = response.json() + assert response_data["error"] == "Vector store ID vs_test_999 not found in OpenAI." + + +def test_update_assistant_success( + client: TestClient, + db: Session, +): + """Test successful assistant update.""" + update_payload = { + "name": "Updated Assistant", + "instructions": "Updated instructions.", + "model": "gpt-4o", + "temperature": 0.7, + "max_num_results": 5, + } + + assistant = get_assistant(db) + api_key = get_api_keys_by_project(db, assistant.project_id)[0] + + response = client.patch( + f"/api/v1/assistant/{assistant.assistant_id}", + json=update_payload, + headers={"X-API-KEY": f"{api_key.key}"}, + ) + + assert response.status_code == 200 + response_data = response.json() + assert response_data["success"] is True + assert response_data["data"]["name"] == update_payload["name"] + assert response_data["data"]["instructions"] == update_payload["instructions"] + assert response_data["data"]["model"] == update_payload["model"] + assert response_data["data"]["temperature"] == update_payload["temperature"] + assert response_data["data"]["max_num_results"] == update_payload["max_num_results"] + + +@patch("app.crud.assistants.verify_vector_store_ids_exist") +def test_update_assistant_invalid_vector_store( + mock_verify_vector_ids, + client: TestClient, + db: Session, +): + """Test failure when updating assistant with invalid vector store IDs.""" + mock_verify_vector_ids.side_effect = HTTPException( + status_code=400, detail="Vector store ID vs_invalid not found in OpenAI." + ) + + update_payload = {"vector_store_ids_add": ["vs_invalid"]} + + assistant = get_assistant(db) + api_key = get_api_keys_by_project(db, assistant.project_id)[0] + + response = client.patch( + f"/api/v1/assistant/{assistant.assistant_id}", + json=update_payload, + headers={"X-API-KEY": f"{api_key.key}"}, + ) + + assert response.status_code == 400 + response_data = response.json() + assert response_data["error"] == "Vector store ID vs_invalid not found in OpenAI." + + +def test_update_assistant_not_found( + client: TestClient, + normal_user_api_key_headers: dict, +): + """Test failure when updating a non-existent assistant.""" + update_payload = {"name": "Updated Assistant"} + + non_existent_id = str(uuid4()) + + response = client.patch( + f"/api/v1/assistant/{non_existent_id}", + json=update_payload, + headers=normal_user_api_key_headers, + ) + + assert response.status_code == 404 + response_data = response.json() + assert "not found" in response_data["error"].lower() + + +def test_get_assistant_success( + client: TestClient, + db: Session, +): + """Test successful retrieval of a single assistant.""" + assistant = get_assistant(db) + api_key = get_api_keys_by_project(db, assistant.project_id)[0] + + response = client.get( + f"/api/v1/assistant/{assistant.assistant_id}", + headers={"X-API-KEY": f"{api_key.key}"}, + ) + + assert response.status_code == 200 + response_data = response.json() + assert response_data["success"] is True + assert response_data["data"]["assistant_id"] == assistant.assistant_id + assert response_data["data"]["name"] == assistant.name + assert response_data["data"]["instructions"] == assistant.instructions + assert response_data["data"]["model"] == assistant.model + + +def test_get_assistant_not_found( + client: TestClient, + normal_user_api_key_headers: dict, +): + """Test failure when fetching a non-existent assistant.""" + non_existent_id = str(uuid4()) + + response = client.get( + f"/api/v1/assistant/{non_existent_id}", + headers=normal_user_api_key_headers, + ) + + assert response.status_code == 404 + response_data = response.json() + assert f"Assistant with ID {non_existent_id} not found." in response_data["error"] + + +def test_list_assistants_success( + client: TestClient, + db: Session, +): + """Test successful retrieval of assistants list.""" + assistant = get_assistant(db) + api_key = get_api_keys_by_project(db, assistant.project_id)[0] + + response = client.get( + "/api/v1/assistant/", + headers={"X-API-KEY": f"{api_key.key}"}, + ) + + assert response.status_code == 200 + response_data = response.json() + assert response_data["success"] is True + assert isinstance(response_data["data"], list) + assert len(response_data["data"]) >= 1 + + # Check that our test assistant is in the list + assistant_ids = [a["assistant_id"] for a in response_data["data"]] + assert assistant.assistant_id in assistant_ids + + +def test_list_assistants_invalid_pagination( + client: TestClient, + normal_user_api_key_headers: dict, +): + """Test assistants list with invalid pagination parameters.""" + # Test negative skip + response = client.get( + "/api/v1/assistant/?skip=-1&limit=10", + headers=normal_user_api_key_headers, + ) + assert response.status_code == 422 + + # Test limit too high + response = client.get( + "/api/v1/assistant/?skip=0&limit=101", + headers=normal_user_api_key_headers, + ) + assert response.status_code == 422 + + # Test limit too low + response = client.get( + "/api/v1/assistant/?skip=0&limit=0", + headers=normal_user_api_key_headers, + ) + assert response.status_code == 422 + + +def test_delete_assistant_success( + client: TestClient, + db: Session, +): + """Test successful soft deletion of an assistant.""" + assistant = get_assistant(db) + api_key = get_api_keys_by_project(db, assistant.project_id)[0] + + response = client.delete( + f"/api/v1/assistant/{assistant.assistant_id}", + headers={"X-API-KEY": f"{api_key.key}"}, + ) + + assert response.status_code == 200 + response_data = response.json() + assert response_data["success"] is True + assert response_data["data"]["message"] == "Assistant deleted successfully." + + +def test_delete_assistant_not_found( + client: TestClient, + normal_user_api_key_headers: dict, +): + """Test failure when deleting a non-existent assistant.""" + non_existent_id = str(uuid4()) + + response = client.delete( + f"/api/v1/assistant/{non_existent_id}", + headers=normal_user_api_key_headers, + ) + + assert response.status_code == 404 + response_data = response.json() + assert "not found" in response_data["error"].lower() diff --git a/backend/app/tests/api/routes/test_responses.py b/backend/app/tests/api/routes/test_responses.py index 19e5df5f..242e6fdc 100644 --- a/backend/app/tests/api/routes/test_responses.py +++ b/backend/app/tests/api/routes/test_responses.py @@ -37,20 +37,21 @@ def test_responses_endpoint_success( mock_response.output = [] mock_client.responses.create.return_value = mock_response - # Get the Glific project ID (the assistant is created for this project) - glific_project = db.exec(select(Project).where(Project.name == "Glific")).first() - if not glific_project: - pytest.skip("Glific project not found in the database") + # Get the Dalgo project ID (the assistant is created for this project) + 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_glific", - "question": "What is Glific?", + "assistant_id": "assistant_dalgo", + "question": "What is Dalgo?", "callback_url": "http://example.com/callback", } response = client.post( "/responses", json=request_data, headers=normal_user_api_key_headers ) + assert response.status_code == 200 response_json = response.json() assert response_json["success"] is True diff --git a/backend/app/tests/crud/test_assistants.py b/backend/app/tests/crud/test_assistants.py index 585aa025..322eed77 100644 --- a/backend/app/tests/crud/test_assistants.py +++ b/backend/app/tests/crud/test_assistants.py @@ -1,13 +1,29 @@ import pytest -from sqlmodel import Session + from fastapi import HTTPException +from sqlmodel import Session +from openai import OpenAI +from unittest.mock import patch +from app.models.project import Project +from app.models import AssistantCreate, AssistantUpdate +from app.crud.assistants import ( + sync_assistant, + create_assistant, + update_assistant, + delete_assistant, + get_assistant_by_id, + get_assistants_by_project, +) from app.tests.utils.openai import mock_openai_assistant -from app.tests.utils.utils import get_project -from app.crud.assistants import sync_assistant +from app.tests.utils.utils import ( + get_project, + get_assistant, + get_non_existent_id, +) -class TestAssistant: +class TestSyncAssistant: def test_sync_assistant_success(self, db: Session): project = get_project(db) openai_assistant = mock_openai_assistant( @@ -113,3 +129,276 @@ def test_sync_assistant_no_tool_resources(self, db: Session): assert result.max_num_results == 20 assert result.assistant_id == openai_assistant.id assert result.project_id == project.id + + +class TestAssistantCrud: + @patch("app.crud.assistants.verify_vector_store_ids_exist") + def test_create_assistant_success(self, mock_vector_store_ids_exist, db: Session): + """Assistant is created when vector store IDs are valid""" + project = get_project(db) + assistant_create = AssistantCreate( + name="Test Assistant", + instructions="Test instructions", + model="gpt-4o", + vector_store_ids=["vs_1", "vs_2"], + temperature=0.7, + max_num_results=10, + ) + client = OpenAI(api_key="test_key") + mock_vector_store_ids_exist.return_value = None + result = create_assistant( + db, client, assistant_create, project.id, project.organization_id + ) + + assert result.name == assistant_create.name + assert result.instructions == assistant_create.instructions + assert result.model == assistant_create.model + assert result.vector_store_ids == assistant_create.vector_store_ids + assert result.temperature == assistant_create.temperature + assert result.max_num_results == assistant_create.max_num_results + + @patch("app.crud.assistants.verify_vector_store_ids_exist") + def test_create_assistant_vector_store_invalid( + self, mock_vector_store_ids_exist, db: Session + ): + """Should raise HTTPException when vector store IDs are invalid""" + project = get_project(db) + assistant_create = AssistantCreate( + name="Invalid VS Assistant", + instructions="Some instructions", + model="gpt-4o", + vector_store_ids=["invalid_vs"], + temperature=0.5, + max_num_results=5, + ) + client = OpenAI(api_key="test_key") + error_message = "Vector store ID vs_test_999 not found in OpenAI." + mock_vector_store_ids_exist.side_effect = HTTPException( + status_code=400, detail=error_message + ) + + with pytest.raises(HTTPException) as exc_info: + create_assistant( + db, client, assistant_create, project.id, project.organization_id + ) + + assert exc_info.value.status_code == 400 + assert error_message in exc_info.value.detail + + @patch("app.crud.assistants.verify_vector_store_ids_exist") + def test_update_assistant_success(self, mock_verify_vector_store_ids, db: Session): + """Assistant is updated successfully with valid data""" + assistant = get_assistant(db) + + assistant_update = AssistantUpdate( + name="Updated Assistant", + instructions="Updated instructions", + temperature=0.9, + max_num_results=15, + vector_store_ids_add=["vs_2"], + ) + mock_verify_vector_store_ids.return_value = None + + client = OpenAI(api_key="test_key") + result = update_assistant( + db, client, assistant.assistant_id, assistant.project_id, assistant_update + ) + + assert result.name == assistant_update.name + assert result.instructions == assistant_update.instructions + assert result.temperature == assistant_update.temperature + assert result.max_num_results == assistant_update.max_num_results + assert "vs_2" in result.vector_store_ids + assert mock_verify_vector_store_ids.called + + def test_update_assistant_not_found(self, db: Session): + """Should raise HTTPException when assistant is not found""" + project = get_project(db) + assistant_id = "non_existent_assistant_id" + assistant_update = AssistantUpdate(name="Updated Assistant") + + client = OpenAI(api_key="test_key") + with pytest.raises(HTTPException) as exc_info: + update_assistant(db, client, assistant_id, project.id, assistant_update) + + assert exc_info.value.status_code == 404 + assert "Assistant not found" in exc_info.value.detail + + @patch("app.crud.assistants.verify_vector_store_ids_exist") + def test_update_assistant_invalid_vector_store( + self, mock_verify_vector_store_ids, db: Session + ): + """Should raise HTTPException when vector store IDs are invalid""" + assistant = get_assistant(db) + error_message = "Vector store ID vs_invalid not found in OpenAI." + mock_verify_vector_store_ids.side_effect = HTTPException( + status_code=400, detail=error_message + ) + assistant_update = AssistantUpdate(vector_store_ids_add=["vs_invalid"]) + + client = OpenAI(api_key="test_key") + with pytest.raises(HTTPException) as exc_info: + update_assistant( + db, + client, + assistant.assistant_id, + assistant.project_id, + assistant_update, + ) + + assert exc_info.value.status_code == 400 + assert error_message in exc_info.value.detail + + def test_update_assistant_conflicting_vector_store_ids(self, db: Session): + """Should raise HTTPException with 400 when vector store IDs are both added and removed""" + assistant = get_assistant(db) + conflicting_id = "vs_1" + assistant_update = AssistantUpdate( + vector_store_ids_add=[conflicting_id], + vector_store_ids_remove=[conflicting_id], + ) + + client = OpenAI(api_key="test_key") + + with pytest.raises(HTTPException) as exc_info: + update_assistant( + session=db, + openai_client=client, + assistant_id=assistant.assistant_id, + project_id=assistant.project_id, + assistant_update=assistant_update, + ) + + expected_error = ( + f"Conflicting vector store IDs in add/remove: {{{conflicting_id!r}}}." + ) + assert exc_info.value.status_code == 400 + assert exc_info.value.detail == expected_error + + @patch("app.crud.assistants.verify_vector_store_ids_exist") + def test_update_assistant_partial_update( + self, mock_verify_vector_store_ids, db: Session + ): + """Assistant is updated successfully with partial fields""" + assistant = get_assistant(db) + mock_verify_vector_store_ids.return_value = None + assistant_update = AssistantUpdate( + name="Partially Updated Assistant", vector_store_ids_add=["vs_3"] + ) + + client = OpenAI(api_key="test_key") + result = update_assistant( + db, client, assistant.assistant_id, assistant.project_id, assistant_update + ) + + assert result.name == assistant_update.name + assert result.instructions == assistant.instructions + assert result.temperature == assistant.temperature + assert result.max_num_results == assistant.max_num_results + assert "vs_3" in result.vector_store_ids + assert mock_verify_vector_store_ids.called + + def test_delete_assistant_success(self, db: Session): + """Assistant is soft deleted successfully""" + assistant = get_assistant(db) + + result = delete_assistant(db, assistant.assistant_id, assistant.project_id) + + assert result.is_deleted is True + assert result.deleted_at is not None + with pytest.raises(ValueError) as exc_info: + get_assistant(db, name=assistant.name) + assert "No active assistants found" in str(exc_info.value) + + def test_delete_assistant_not_found(self, db: Session): + """Should raise HTTPException when assistant is not found""" + project = get_project(db) + assistant_id = "non_existent_assistant_id" + + with pytest.raises(HTTPException) as exc_info: + delete_assistant(db, assistant_id, project.id) + + assert exc_info.value.status_code == 404 + assert "Assistant not found" in exc_info.value.detail + + def test_get_assistant_by_id_success(self, db: Session): + """Assistant is retrieved successfully by ID and project ID""" + assistant = get_assistant(db) + + result = get_assistant_by_id(db, assistant.assistant_id, assistant.project_id) + + assert result is not None + assert result.assistant_id == assistant.assistant_id + assert result.project_id == assistant.project_id + assert result.is_deleted is False + + def test_get_assistant_by_id_not_found(self, db: Session): + """Returns None when assistant is not found""" + project = get_project(db) + non_existent_id = "NonExistentAssistantId" + + result = get_assistant_by_id(db, non_existent_id, project.id) + + assert result is None + + def test_get_assistant_by_id_deleted(self, db: Session): + """Returns None when assistant is soft deleted""" + assistant = get_assistant(db) + # Soft delete the assistant + delete_assistant(db, assistant.assistant_id, assistant.project_id) + + result = get_assistant_by_id(db, assistant.assistant_id, assistant.project_id) + + assert result is None + + @patch("app.crud.assistants.verify_vector_store_ids_exist") + def test_get_assistants_by_project_success( + self, mock_vector_store_ids_exist, db: Session + ): + """Returns all assistants for a project""" + project = get_project(db) + client = OpenAI(api_key="test_key") + mock_vector_store_ids_exist.return_value = None + + assistant_create1 = AssistantCreate( + name="Assistant 1", + instructions="Instructions 1", + model="gpt-4o", + vector_store_ids=["vs_1", "vs_2"], + temperature=0.5, + max_num_results=10, + ) + assistant_create2 = AssistantCreate( + name="Assistant 2", + instructions="Instructions 2", + model="gpt-4o", + vector_store_ids=["vs_3"], + temperature=0.6, + max_num_results=20, + ) + + assistant1 = create_assistant( + db, client, assistant_create1, project.id, project.organization_id + ) + assistant2 = create_assistant( + db, client, assistant_create2, project.id, project.organization_id + ) + + result = get_assistants_by_project(db, project.id) + + assert len(result) >= 2 + assistant_ids = [a.assistant_id for a in result] + assert assistant1.assistant_id in assistant_ids + assert assistant2.assistant_id in assistant_ids + for assistant in result: + assert assistant.project_id == project.id + assert assistant.is_deleted is False + + def test_get_assistants_by_project_empty(self, db: Session): + """Returns empty list when project has no assistants""" + project = get_project(db) + non_existent_project_id = get_non_existent_id(db, Project) + + result = get_assistants_by_project(db, non_existent_project_id) + + assert result == [] diff --git a/backend/app/tests/utils/utils.py b/backend/app/tests/utils/utils.py index 92a714b0..505d3e20 100644 --- a/backend/app/tests/utils/utils.py +++ b/backend/app/tests/utils/utils.py @@ -9,9 +9,9 @@ from sqlmodel import Session, select from app.core.config import settings -from app.crud import get_user_by_email, get_api_key_by_user_id -from app.models import APIKeyPublic, Project -from app.crud import get_api_key_by_value +from app.crud.user import get_user_by_email +from app.crud.api_key import get_api_key_by_value, get_api_key_by_user_id +from app.models import APIKeyPublic, Project, Assistant T = TypeVar("T") @@ -93,6 +93,30 @@ def get_project(session: Session, name: str | None = None) -> Project: return project +def get_assistant(session: Session, name: str | None = None) -> Assistant: + """ + Retrieve an active assistant from the database. + + If a assistant name is provided, fetch the active assistant with that name. + If no name is provided, fetch any random assistant. + """ + if name: + statement = ( + select(Assistant) + .where(Assistant.name == name, Assistant.is_deleted == False) + .limit(1) + ) + else: + statement = select(Assistant).where(Assistant.is_deleted == False).limit(1) + + assistant = session.exec(statement).first() + + if not assistant: + raise ValueError("No active assistants found") + + return assistant + + class SequentialUuidGenerator: def __init__(self, start=0): self.start = start diff --git a/backend/app/utils.py b/backend/app/utils.py index 8f5be811..38f8c650 100644 --- a/backend/app/utils.py +++ b/backend/app/utils.py @@ -177,6 +177,9 @@ def get_openai_client(session: Session, org_id: int, project_id: int) -> OpenAI: ) if not credentials or "api_key" not in credentials: + logger.warning( + f"[get_openai_client] OpenAI credentials not found. | project_id: {project_id}" + ) raise HTTPException( status_code=400, detail="OpenAI credentials not configured for this organization/project.", @@ -185,6 +188,10 @@ def get_openai_client(session: Session, org_id: int, project_id: int) -> OpenAI: try: return OpenAI(api_key=credentials["api_key"]) except Exception as e: + logger.error( + f"[get_openai_client] Failed to configure OpenAI client. | project_id: {project_id} | error: {str(e)}", + exc_info=True, + ) raise HTTPException( status_code=500, detail=f"Failed to configure OpenAI client: {str(e)}",