From 288d916999a77f75bf7d26d34483ac0b1cc881e2 Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Fri, 18 Jul 2025 14:29:08 +0530 Subject: [PATCH 01/17] create api for assistant and get_assistant_by_id to use project id --- backend/app/api/routes/assistants.py | 25 ++++++++++++++- backend/app/api/routes/responses.py | 2 +- backend/app/crud/__init__.py | 1 + backend/app/crud/assistants.py | 46 +++++++++++++++++++++++++--- backend/app/models/__init__.py | 2 +- backend/app/models/assistants.py | 34 ++++++++++++++++++-- 6 files changed, 100 insertions(+), 10 deletions(-) diff --git a/backend/app/api/routes/assistants.py b/backend/app/api/routes/assistants.py index 35f66146..826be82a 100644 --- a/backend/app/api/routes/assistants.py +++ b/backend/app/api/routes/assistants.py @@ -7,8 +7,9 @@ from app.crud import ( fetch_assistant_from_openai, sync_assistant, + create_assistant ) -from app.models import UserProjectOrg +from app.models import UserProjectOrg, AssistantCreate from app.utils import APIResponse, get_openai_client router = APIRouter(prefix="/assistant", tags=["Assistants"]) @@ -41,3 +42,25 @@ def ingest_assistant_route( ) return APIResponse.success_response(assistant) + + +@router.post("/", response_model=APIResponse, 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) \ No newline at end of file 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 6ec996c2..45226767 100644 --- a/backend/app/crud/__init__.py +++ b/backend/app/crud/__init__.py @@ -48,4 +48,5 @@ get_assistant_by_id, fetch_assistant_from_openai, sync_assistant, + create_assistant ) diff --git a/backend/app/crud/assistants.py b/backend/app/crud/assistants.py index 3f040508..7bbb27f9 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,20 +9,20 @@ 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 from app.utils import mask_string 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, ) ) return session.exec(statement).first() @@ -48,6 +49,21 @@ 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 vs_id in vector_store_ids: + try: + openai_client.vector_stores.retrieve(vs_id) + except Exception as e: + logger.error(f"Vector store id {vs_id} not found in OpenAI: {e}") + raise HTTPException( + status_code=400, + detail=f"Vector store ID {vs_id} not found in OpenAI." + ) + + def sync_assistant( session: Session, organization_id: int, @@ -59,7 +75,7 @@ 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." @@ -111,3 +127,25 @@ 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= uuid.uuid4(), + **assistant.model_dump(), + project_id=project_id, + organization_id=organization_id, + ) + session.add(assistant) + session.commit() + session.refresh(assistant) + return assistant \ No newline at end of file diff --git a/backend/app/models/__init__.py b/backend/app/models/__init__.py index 04693637..993722f3 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 diff --git a/backend/app/models/assistants.py b/backend/app/models/assistants.py index 5647455d..28972026 100644 --- a/backend/app/models/assistants.py +++ b/backend/app/models/assistants.py @@ -1,11 +1,14 @@ from datetime import datetime -from typing import Optional, List -from sqlmodel import Field, Relationship, SQLModel -from sqlalchemy import Column, String +from typing import List, Optional +from sqlmodel import Field, Relationship, SQLModel, Column, String from sqlalchemy.dialects.postgresql import ARRAY +from pydantic import BaseModel, Field as PydanticField, field_validator + from app.core.util import now +ALLOWED_OPENAI_MODELS = {"gpt-3.5-turbo", "gpt-4", "gpt-4o"} + class AssistantBase(SQLModel): assistant_id: str = Field(index=True, unique=True) @@ -31,3 +34,28 @@ class Assistant(AssistantBase, table=True): # Relationships project: "Project" = Relationship(back_populates="assistants") organization: "Organization" = Relationship(back_populates="assistants") + + +class AssistantCreate(BaseModel): + name: str = PydanticField(description="Name of the assistant") + instructions: str = PydanticField(description="Instructions for the assistant") + model: str = PydanticField(description="Model name for the assistant") + vector_store_ids: List[str] = PydanticField(default_factory=list, description="List of Vector Store IDs that exist in OpenAI.") + temperature: Optional[float] = PydanticField( + default=0.1, + ge=0, + le=2, + description="Sampling temperature between 0 and 2" + ) + max_num_results: Optional[int] = PydanticField( + default=20, + ge=1, + le=100, + description="Maximum number of results (must be between 1 and 100)" + ) + + @field_validator("model") + def validate_openai_model(cls, v): + if v not in ALLOWED_OPENAI_MODELS: + raise ValueError(f"Model '{v}' is not a supported OpenAI model. Choose from: {', '.join(ALLOWED_OPENAI_MODELS)}") + return v From 78629e5c74deacf0c700f126ff23a84701b7f75a Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Fri, 18 Jul 2025 15:02:31 +0530 Subject: [PATCH 02/17] Update API for assistant --- backend/app/api/routes/assistants.py | 35 +++++++++++-- backend/app/crud/__init__.py | 3 +- backend/app/crud/assistants.py | 76 ++++++++++++++++++++++++---- backend/app/models/__init__.py | 2 +- backend/app/models/assistants.py | 45 +++++++++++++--- 5 files changed, 137 insertions(+), 24 deletions(-) diff --git a/backend/app/api/routes/assistants.py b/backend/app/api/routes/assistants.py index 826be82a..94bcae25 100644 --- a/backend/app/api/routes/assistants.py +++ b/backend/app/api/routes/assistants.py @@ -7,9 +7,10 @@ from app.crud import ( fetch_assistant_from_openai, sync_assistant, - create_assistant + create_assistant, + update_assistant, ) -from app.models import UserProjectOrg, AssistantCreate +from app.models import UserProjectOrg, AssistantCreate, AssistantUpdate, Assistant from app.utils import APIResponse, get_openai_client router = APIRouter(prefix="/assistant", tags=["Assistants"]) @@ -17,7 +18,7 @@ @router.post( "/{assistant_id}/ingest", - response_model=APIResponse, + response_model=APIResponse[Assistant], status_code=201, ) def ingest_assistant_route( @@ -44,7 +45,7 @@ def ingest_assistant_route( return APIResponse.success_response(assistant) -@router.post("/", response_model=APIResponse, status_code=201) +@router.post("/", response_model=APIResponse[Assistant], status_code=201) def create_assistant_route( assistant_in: AssistantCreate, session: Session = Depends(get_db), @@ -63,4 +64,28 @@ def create_assistant_route( project_id=current_user.project_id, organization_id=current_user.organization_id, ) - return APIResponse.success_response(assistant) \ No newline at end of file + return APIResponse.success_response(assistant) + + +@router.put("/{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, + organization_id=current_user.organization_id, + assistant_update=assistant_update, + ) + return APIResponse.success_response(updated_assistant) diff --git a/backend/app/crud/__init__.py b/backend/app/crud/__init__.py index 45226767..3b6496f6 100644 --- a/backend/app/crud/__init__.py +++ b/backend/app/crud/__init__.py @@ -48,5 +48,6 @@ get_assistant_by_id, fetch_assistant_from_openai, sync_assistant, - create_assistant + create_assistant, + update_assistant, ) diff --git a/backend/app/crud/assistants.py b/backend/app/crud/assistants.py index 7bbb27f9..b864960f 100644 --- a/backend/app/crud/assistants.py +++ b/backend/app/crud/assistants.py @@ -1,6 +1,6 @@ import logging -from typing import Optional +from typing import Optional, Set import uuid import openai @@ -9,8 +9,9 @@ from openai.types.beta import Assistant as OpenAIAssistant from sqlmodel import Session, and_, select -from app.models import Assistant, AssistantCreate +from app.models import Assistant, AssistantCreate, AssistantUpdate from app.utils import mask_string +from app.core.util import now logger = logging.getLogger(__name__) @@ -49,7 +50,9 @@ 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: +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. """ @@ -59,8 +62,7 @@ def verify_vector_store_ids_exist(openai_client: OpenAI, vector_store_ids: list[ except Exception as e: logger.error(f"Vector store id {vs_id} not found in OpenAI: {e}") raise HTTPException( - status_code=400, - detail=f"Vector store ID {vs_id} not found in OpenAI." + status_code=400, detail=f"Vector store ID {vs_id} not found in OpenAI." ) @@ -136,11 +138,10 @@ def create_assistant( project_id: int, organization_id: int, ) -> Assistant: - verify_vector_store_ids_exist(openai_client, assistant.vector_store_ids) - + assistant = Assistant( - assistant_id= uuid.uuid4(), + assistant_id=uuid.uuid4(), **assistant.model_dump(), project_id=project_id, organization_id=organization_id, @@ -148,4 +149,61 @@ def create_assistant( session.add(assistant) session.commit() session.refresh(assistant) - return assistant \ No newline at end of file + return assistant + + +def update_assistant( + session: Session, + openai_client: OpenAI, + assistant_id: str, + project_id: int, + organization_id: int, + assistant_update: AssistantUpdate, +) -> Assistant: + existing_assistant = get_assistant_by_id(session, assistant_id, organization_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"Conflicting vector store IDs in add/remove: {conflicting_ids} | project_id: {project_id}" + ) + raise ValueError( + f"Cannot add and remove the same vector store IDs: {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 diff --git a/backend/app/models/__init__.py b/backend/app/models/__init__.py index 993722f3..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, AssistantCreate +from .assistants import Assistant, AssistantBase, AssistantCreate, AssistantUpdate diff --git a/backend/app/models/assistants.py b/backend/app/models/assistants.py index 28972026..3804c396 100644 --- a/backend/app/models/assistants.py +++ b/backend/app/models/assistants.py @@ -39,23 +39,52 @@ class Assistant(AssistantBase, table=True): class AssistantCreate(BaseModel): name: str = PydanticField(description="Name of the assistant") instructions: str = PydanticField(description="Instructions for the assistant") - model: str = PydanticField(description="Model name for the assistant") - vector_store_ids: List[str] = PydanticField(default_factory=list, description="List of Vector Store IDs that exist in OpenAI.") + model: str = PydanticField( + default="gpt-4o", description="Model name for the assistant" + ) + vector_store_ids: List[str] = PydanticField( + default_factory=list, + description="List of Vector Store IDs that exist in OpenAI.", + ) temperature: Optional[float] = PydanticField( - default=0.1, - ge=0, - le=2, - description="Sampling temperature between 0 and 2" + default=0.1, ge=0, le=2, description="Sampling temperature between 0 and 2" ) max_num_results: Optional[int] = PydanticField( default=20, ge=1, le=100, - description="Maximum number of results (must be between 1 and 100)" + description="Maximum number of results (must be between 1 and 100)", + ) + + @field_validator("model") + def validate_openai_model(cls, v): + if v not in ALLOWED_OPENAI_MODELS: + raise ValueError( + f"Model '{v}' is not a supported OpenAI model. Choose from: {', '.join(ALLOWED_OPENAI_MODELS)}" + ) + return v + + +class AssistantUpdate(BaseModel): + name: Optional[str] = None + instructions: Optional[str] = None + model: Optional[str] = None + vector_store_ids_add: Optional[List[str]] = None + vector_store_ids_remove: Optional[List[str]] = None + temperature: Optional[float] = PydanticField( + default=None, ge=0, le=2, description="Sampling temperature between 0 and 2" + ) + max_num_results: Optional[int] = PydanticField( + default=None, + ge=1, + le=100, + description="Maximum number of results (must be between 1 and 100)", ) @field_validator("model") def validate_openai_model(cls, v): if v not in ALLOWED_OPENAI_MODELS: - raise ValueError(f"Model '{v}' is not a supported OpenAI model. Choose from: {', '.join(ALLOWED_OPENAI_MODELS)}") + raise ValueError( + f"Model '{v}' is not a supported OpenAI model. Choose from: {', '.join(ALLOWED_OPENAI_MODELS)}" + ) return v From daa202e51af6e7d9fe0dcee042dfbcca1f9de9d1 Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Fri, 18 Jul 2025 15:13:22 +0530 Subject: [PATCH 03/17] list endpoint for assistant --- backend/app/api/routes/assistants.py | 80 +++++++++++++++++++++++++++- backend/app/crud/__init__.py | 1 + backend/app/crud/assistants.py | 23 +++++++- 3 files changed, 102 insertions(+), 2 deletions(-) diff --git a/backend/app/api/routes/assistants.py b/backend/app/api/routes/assistants.py index 94bcae25..a797068f 100644 --- a/backend/app/api/routes/assistants.py +++ b/backend/app/api/routes/assistants.py @@ -1,6 +1,6 @@ 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 @@ -9,6 +9,8 @@ sync_assistant, create_assistant, update_assistant, + get_assistant_by_id, + get_assistants_by_project ) from app.models import UserProjectOrg, AssistantCreate, AssistantUpdate, Assistant from app.utils import APIResponse, get_openai_client @@ -89,3 +91,79 @@ def update_assistant_route( 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( + "/{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, + 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) \ No newline at end of file diff --git a/backend/app/crud/__init__.py b/backend/app/crud/__init__.py index 3b6496f6..21111b70 100644 --- a/backend/app/crud/__init__.py +++ b/backend/app/crud/__init__.py @@ -50,4 +50,5 @@ sync_assistant, create_assistant, update_assistant, + get_assistants_by_project ) diff --git a/backend/app/crud/assistants.py b/backend/app/crud/assistants.py index b864960f..f0536b42 100644 --- a/backend/app/crud/assistants.py +++ b/backend/app/crud/assistants.py @@ -29,6 +29,27 @@ def get_assistant_by_id( 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 + ) + .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. @@ -160,7 +181,7 @@ def update_assistant( organization_id: int, assistant_update: AssistantUpdate, ) -> Assistant: - existing_assistant = get_assistant_by_id(session, assistant_id, organization_id) + 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}" From b90b6411209967e6cc4ed8b2ac3bdf10539a058b Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Fri, 18 Jul 2025 15:47:31 +0530 Subject: [PATCH 04/17] delete endpoint for assistant crud --- ...dd_is_deleted_column_in_assistant_table.py | 39 +++++++++++ backend/app/api/routes/assistants.py | 66 ++++++++----------- backend/app/crud/__init__.py | 3 +- backend/app/crud/assistants.py | 33 +++++++++- backend/app/models/assistants.py | 2 + 5 files changed, 100 insertions(+), 43 deletions(-) create mode 100644 backend/app/alembic/versions/d4af4420f652_add_is_deleted_column_in_assistant_table.py diff --git a/backend/app/alembic/versions/d4af4420f652_add_is_deleted_column_in_assistant_table.py b/backend/app/alembic/versions/d4af4420f652_add_is_deleted_column_in_assistant_table.py new file mode 100644 index 00000000..fac6be6b --- /dev/null +++ b/backend/app/alembic/versions/d4af4420f652_add_is_deleted_column_in_assistant_table.py @@ -0,0 +1,39 @@ +"""Add is_deleted column in assistant table + +Revision ID: d4af4420f652 +Revises: f2589428c1d0 +Create Date: 2025-07-18 15:14:45.700564 + +""" +from alembic import op +import sqlalchemy as sa +import sqlmodel.sql.sqltypes + + +# revision identifiers, used by Alembic. +revision = 'd4af4420f652' +down_revision = 'f2589428c1d0' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column( + 'openai_assistant', + sa.Column('is_deleted', sa.Boolean(), nullable=False, server_default=sa.text('FALSE')) + ) + op.add_column( + 'openai_assistant', + sa.Column('deleted_at', sa.DateTime(), nullable=True) + ) + # Remove the default (optional, to avoid future auto-defaults) + op.alter_column('openai_assistant', 'is_deleted', server_default=None) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('openai_assistant', 'deleted_at') + op.drop_column('openai_assistant', 'is_deleted') + # ### end Alembic commands ### diff --git a/backend/app/api/routes/assistants.py b/backend/app/api/routes/assistants.py index a797068f..93d6388d 100644 --- a/backend/app/api/routes/assistants.py +++ b/backend/app/api/routes/assistants.py @@ -10,7 +10,8 @@ create_assistant, update_assistant, get_assistant_by_id, - get_assistants_by_project + get_assistants_by_project, + delete_assistant, ) from app.models import UserProjectOrg, AssistantCreate, AssistantUpdate, Assistant from app.utils import APIResponse, get_openai_client @@ -101,54 +102,23 @@ def update_assistant_route( 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) + 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 - ) + 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." + status_code=404, detail=f"Assistant with ID {assistant_id} not found." ) return APIResponse.success_response(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, - summary="List all assistants in the current project" + response_model=APIResponse[list[Assistant]], + summary="List all assistants in the current project", ) def list_assistants_route( session: Session = Depends(get_db), @@ -161,9 +131,25 @@ def list_assistants_route( """ 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, - skip=skip, - limit=limit ) - return APIResponse.success_response(assistants) \ No newline at end of file + return APIResponse.success_response( + data={"message": "Assistant deleted successfully."} + ) diff --git a/backend/app/crud/__init__.py b/backend/app/crud/__init__.py index 21111b70..25cb73e8 100644 --- a/backend/app/crud/__init__.py +++ b/backend/app/crud/__init__.py @@ -50,5 +50,6 @@ sync_assistant, create_assistant, update_assistant, - get_assistants_by_project + get_assistants_by_project, + delete_assistant, ) diff --git a/backend/app/crud/assistants.py b/backend/app/crud/assistants.py index f0536b42..9aac21fe 100644 --- a/backend/app/crud/assistants.py +++ b/backend/app/crud/assistants.py @@ -24,6 +24,7 @@ def get_assistant_by_id( and_( Assistant.assistant_id == assistant_id, Assistant.project_id == project_id, + Assistant.is_deleted == False, ) ) return session.exec(statement).first() @@ -41,10 +42,11 @@ def get_assistants_by_project( statement = ( select(Assistant) .where( - Assistant.project_id == project_id + Assistant.project_id == project_id, + Assistant.is_deleted == False, ) .offset(skip) - .limit(limit) + .limit(limit), ) results = session.exec(statement).all() return results @@ -228,3 +230,30 @@ def update_assistant( 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.error( + 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/assistants.py b/backend/app/models/assistants.py index 3804c396..35919708 100644 --- a/backend/app/models/assistants.py +++ b/backend/app/models/assistants.py @@ -30,6 +30,8 @@ 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") From 6ed1df1363fa239c4fbf1fbcb3f178fe6dfac02d Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Mon, 21 Jul 2025 09:56:35 +0530 Subject: [PATCH 05/17] unit test for assistant crud --- ...dd_is_deleted_column_in_assistant_table.py | 19 +- backend/app/api/routes/assistants.py | 3 +- backend/app/crud/assistants.py | 18 +- .../app/tests/api/routes/test_assistants.py | 296 ++++++++++++++++++ backend/app/tests/crud/test_assistants.py | 290 ++++++++++++++++- backend/app/tests/utils/utils.py | 26 +- 6 files changed, 630 insertions(+), 22 deletions(-) diff --git a/backend/app/alembic/versions/d4af4420f652_add_is_deleted_column_in_assistant_table.py b/backend/app/alembic/versions/d4af4420f652_add_is_deleted_column_in_assistant_table.py index fac6be6b..cfd4d5ec 100644 --- a/backend/app/alembic/versions/d4af4420f652_add_is_deleted_column_in_assistant_table.py +++ b/backend/app/alembic/versions/d4af4420f652_add_is_deleted_column_in_assistant_table.py @@ -11,8 +11,8 @@ # revision identifiers, used by Alembic. -revision = 'd4af4420f652' -down_revision = 'f2589428c1d0' +revision = "d4af4420f652" +down_revision = "f2589428c1d0" branch_labels = None depends_on = None @@ -20,20 +20,21 @@ def upgrade(): # ### commands auto generated by Alembic - please adjust! ### op.add_column( - 'openai_assistant', - sa.Column('is_deleted', sa.Boolean(), nullable=False, server_default=sa.text('FALSE')) + "openai_assistant", + sa.Column( + "is_deleted", sa.Boolean(), nullable=False, server_default=sa.text("FALSE") + ), ) op.add_column( - 'openai_assistant', - sa.Column('deleted_at', sa.DateTime(), nullable=True) + "openai_assistant", sa.Column("deleted_at", sa.DateTime(), nullable=True) ) # Remove the default (optional, to avoid future auto-defaults) - op.alter_column('openai_assistant', 'is_deleted', server_default=None) + op.alter_column("openai_assistant", "is_deleted", server_default=None) # ### end Alembic commands ### def downgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.drop_column('openai_assistant', 'deleted_at') - op.drop_column('openai_assistant', 'is_deleted') + op.drop_column("openai_assistant", "deleted_at") + op.drop_column("openai_assistant", "is_deleted") # ### end Alembic commands ### diff --git a/backend/app/api/routes/assistants.py b/backend/app/api/routes/assistants.py index 93d6388d..3ed327f2 100644 --- a/backend/app/api/routes/assistants.py +++ b/backend/app/api/routes/assistants.py @@ -70,7 +70,7 @@ def create_assistant_route( return APIResponse.success_response(assistant) -@router.put("/{assistant_id}", response_model=APIResponse[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, @@ -88,7 +88,6 @@ def update_assistant_route( assistant_id=assistant_id, openai_client=client, project_id=current_user.project_id, - organization_id=current_user.organization_id, assistant_update=assistant_update, ) return APIResponse.success_response(updated_assistant) diff --git a/backend/app/crud/assistants.py b/backend/app/crud/assistants.py index 9aac21fe..2260c31c 100644 --- a/backend/app/crud/assistants.py +++ b/backend/app/crud/assistants.py @@ -46,7 +46,7 @@ def get_assistants_by_project( Assistant.is_deleted == False, ) .offset(skip) - .limit(limit), + .limit(limit) ) results = session.exec(statement).all() return results @@ -82,10 +82,17 @@ def verify_vector_store_ids_exist( for vs_id in vector_store_ids: try: openai_client.vector_stores.retrieve(vs_id) - except Exception as e: - logger.error(f"Vector store id {vs_id} not found in OpenAI: {e}") + except openai.NotFoundError: + logger.error(f"Vector store ID {vs_id} not found in OpenAI.") raise HTTPException( - status_code=400, detail=f"Vector store ID {vs_id} not found in OpenAI." + status_code=400, + detail=f"Vector store ID {vs_id} not found in OpenAI.", + ) + except openai.OpenAIError as e: + logger.error(f"Failed to verify vector store ID {vs_id}: {e}") + raise HTTPException( + status_code=502, + detail=f"Error verifying vector store ID {vs_id}: {str(e)}", ) @@ -165,7 +172,7 @@ def create_assistant( assistant = Assistant( assistant_id=uuid.uuid4(), - **assistant.model_dump(), + **assistant.model_dump(exclude_unset=True), project_id=project_id, organization_id=organization_id, ) @@ -180,7 +187,6 @@ def update_assistant( openai_client: OpenAI, assistant_id: str, project_id: int, - organization_id: int, assistant_update: AssistantUpdate, ) -> Assistant: existing_assistant = get_assistant_by_id(session, assistant_id, project_id) diff --git a/backend/app/tests/api/routes/test_assistants.py b/backend/app/tests/api/routes/test_assistants.py index 936e0c07..4db0ab9e 100644 --- a/backend/app/tests/api/routes/test_assistants.py +++ b/backend/app/tests/api/routes/test_assistants.py @@ -1,7 +1,12 @@ 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 @@ -9,6 +14,23 @@ def normal_user_api_key_header(): return {"X-API-KEY": "ApiKey Px8y47B6roJHin1lWLkR88eiDrFdXSJRZmFQazzai8j9"} +@pytest.fixture +def assistant_create_payload(): + return { + "name": "Test Assistant", + "instructions": "These are test instructions.", + "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") def test_ingest_assistant_success( mock_fetch_assistant, @@ -29,3 +51,277 @@ 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_header: 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_header, + ) + + 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_header: 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_header, + ) + + 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, + normal_user_api_key_header: dict, +): + """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] + + print(api_key.key) + 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, + normal_user_api_key_header: dict, +): + """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_header: 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_header, + ) + + 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_header: 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_header, + ) + + 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_header: 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_header, + ) + assert response.status_code == 422 + + # Test limit too high + response = client.get( + "/api/v1/assistant/?skip=0&limit=101", + headers=normal_user_api_key_header, + ) + assert response.status_code == 422 + + # Test limit too low + response = client.get( + "/api/v1/assistant/?skip=0&limit=0", + headers=normal_user_api_key_header, + ) + 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_header: 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_header, + ) + + assert response.status_code == 404 + response_data = response.json() + assert "not found" in response_data["error"].lower() diff --git a/backend/app/tests/crud/test_assistants.py b/backend/app/tests/crud/test_assistants.py index 585aa025..503ea154 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,269 @@ 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 ValueError when vector store IDs are both added and removed""" + assistant = get_assistant(db) + assistant_update = AssistantUpdate( + vector_store_ids_add=["vs_1"], vector_store_ids_remove=["vs_1"] + ) + + client = OpenAI(api_key="test_key") + with pytest.raises(ValueError) as exc_info: + update_assistant( + db, + client, + assistant.assistant_id, + assistant.project_id, + assistant_update, + ) + + assert "Cannot add and remove the same vector store IDs" in str(exc_info.value) + + @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 d4812424..e6c14f7d 100644 --- a/backend/app/tests/utils/utils.py +++ b/backend/app/tests/utils/utils.py @@ -10,7 +10,7 @@ from app.core.config import settings from app.crud.user import get_user_by_email from app.crud.api_key import get_api_key_by_value -from app.models import APIKeyPublic, Project +from app.models import APIKeyPublic, Project, Assistant T = TypeVar("T") @@ -85,6 +85,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 From 80b8e27382cab62398285fce73df44fa410c5b36 Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Mon, 21 Jul 2025 12:40:55 +0530 Subject: [PATCH 06/17] Fix migration head by generating new migration --- ...dd_is_deleted_column_in_assistant_table.py | 40 ------------------- ...dd_is_deleted_column_in_assistant_table.py | 30 ++++++++++++++ 2 files changed, 30 insertions(+), 40 deletions(-) delete mode 100644 backend/app/alembic/versions/d4af4420f652_add_is_deleted_column_in_assistant_table.py create mode 100644 backend/app/alembic/versions/e8ee93526b37_add_is_deleted_column_in_assistant_table.py diff --git a/backend/app/alembic/versions/d4af4420f652_add_is_deleted_column_in_assistant_table.py b/backend/app/alembic/versions/d4af4420f652_add_is_deleted_column_in_assistant_table.py deleted file mode 100644 index cfd4d5ec..00000000 --- a/backend/app/alembic/versions/d4af4420f652_add_is_deleted_column_in_assistant_table.py +++ /dev/null @@ -1,40 +0,0 @@ -"""Add is_deleted column in assistant table - -Revision ID: d4af4420f652 -Revises: f2589428c1d0 -Create Date: 2025-07-18 15:14:45.700564 - -""" -from alembic import op -import sqlalchemy as sa -import sqlmodel.sql.sqltypes - - -# revision identifiers, used by Alembic. -revision = "d4af4420f652" -down_revision = "f2589428c1d0" -branch_labels = None -depends_on = None - - -def upgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.add_column( - "openai_assistant", - sa.Column( - "is_deleted", sa.Boolean(), nullable=False, server_default=sa.text("FALSE") - ), - ) - op.add_column( - "openai_assistant", sa.Column("deleted_at", sa.DateTime(), nullable=True) - ) - # Remove the default (optional, to avoid future auto-defaults) - op.alter_column("openai_assistant", "is_deleted", server_default=None) - # ### end Alembic commands ### - - -def downgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.drop_column("openai_assistant", "deleted_at") - op.drop_column("openai_assistant", "is_deleted") - # ### end Alembic commands ### 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..aefd63fb --- /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(): + # ### commands auto generated by Alembic - please adjust! ### + 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)) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('openai_assistant', 'deleted_at') + op.drop_column('openai_assistant', 'is_deleted') + # ### end Alembic commands ### From 287f47ae81ff63929fadb29ae690111cea89cbcd Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Mon, 21 Jul 2025 12:43:14 +0530 Subject: [PATCH 07/17] precommit --- ...7_add_is_deleted_column_in_assistant_table.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) 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 index aefd63fb..a7992fbf 100644 --- 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 @@ -10,21 +10,25 @@ # revision identifiers, used by Alembic. -revision = 'e8ee93526b37' -down_revision = '4aa1f48c6321' +revision = "e8ee93526b37" +down_revision = "4aa1f48c6321" branch_labels = None depends_on = None def upgrade(): # ### commands auto generated by Alembic - please adjust! ### - 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)) + 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) + ) # ### end Alembic commands ### def downgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.drop_column('openai_assistant', 'deleted_at') - op.drop_column('openai_assistant', 'is_deleted') + op.drop_column("openai_assistant", "deleted_at") + op.drop_column("openai_assistant", "is_deleted") # ### end Alembic commands ### From c9b5a3ba31092505fa16030563e66a4ce692b58c Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Mon, 21 Jul 2025 12:52:37 +0530 Subject: [PATCH 08/17] type annotation fix --- backend/app/models/assistants.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/backend/app/models/assistants.py b/backend/app/models/assistants.py index 23fe9274..ec052f51 100644 --- a/backend/app/models/assistants.py +++ b/backend/app/models/assistants.py @@ -50,14 +50,14 @@ class AssistantCreate(BaseModel): model: str = PydanticField( default="gpt-4o", description="Model name for the assistant" ) - vector_store_ids: List[str] = PydanticField( + vector_store_ids: list[str] = PydanticField( default_factory=list, description="List of Vector Store IDs that exist in OpenAI.", ) - temperature: Optional[float] = PydanticField( + temperature: float | None = PydanticField( default=0.1, ge=0, le=2, description="Sampling temperature between 0 and 2" ) - max_num_results: Optional[int] = PydanticField( + max_num_results: int | None = PydanticField( default=20, ge=1, le=100, @@ -74,15 +74,15 @@ def validate_openai_model(cls, v): class AssistantUpdate(BaseModel): - name: Optional[str] = None - instructions: Optional[str] = None - model: Optional[str] = None - vector_store_ids_add: Optional[List[str]] = None - vector_store_ids_remove: Optional[List[str]] = None - temperature: Optional[float] = PydanticField( + name: str | None = None + instructions: str | None = None + model: str | None = None + vector_store_ids_add: list[str] | None = None + vector_store_ids_remove: list[str] | None = None + temperature: float | None = PydanticField( default=None, ge=0, le=2, description="Sampling temperature between 0 and 2" ) - max_num_results: Optional[int] = PydanticField( + max_num_results: int | None = PydanticField( default=None, ge=1, le=100, From e3e1cce319149bf9e460621840996ced8d240576 Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Mon, 21 Jul 2025 13:07:51 +0530 Subject: [PATCH 09/17] code refactor --- backend/app/tests/api/routes/test_assistants.py | 1 - backend/app/tests/api/routes/test_responses.py | 12 +++++------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/backend/app/tests/api/routes/test_assistants.py b/backend/app/tests/api/routes/test_assistants.py index fd980836..47da4b2f 100644 --- a/backend/app/tests/api/routes/test_assistants.py +++ b/backend/app/tests/api/routes/test_assistants.py @@ -130,7 +130,6 @@ def test_update_assistant_success( assistant = get_assistant(db) api_key = get_api_keys_by_project(db, assistant.project_id)[0] - print(api_key.key) response = client.patch( f"/api/v1/assistant/{assistant.assistant_id}", json=update_payload, diff --git a/backend/app/tests/api/routes/test_responses.py b/backend/app/tests/api/routes/test_responses.py index 0e7a2ab6..242e6fdc 100644 --- a/backend/app/tests/api/routes/test_responses.py +++ b/backend/app/tests/api/routes/test_responses.py @@ -37,10 +37,10 @@ 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 == "Dalgo")).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_dalgo", @@ -48,12 +48,10 @@ def test_responses_endpoint_success( "callback_url": "http://example.com/callback", } - # import time - # time.sleep(2000) response = client.post( "/responses", json=request_data, headers=normal_user_api_key_headers ) - print(response.json()) # Debugging output + assert response.status_code == 200 response_json = response.json() assert response_json["success"] is True From 29465a17943ea5f9c86a78125c31e109e543b942 Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Mon, 21 Jul 2025 18:10:08 +0530 Subject: [PATCH 10/17] Update request validation --- backend/app/models/assistants.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/backend/app/models/assistants.py b/backend/app/models/assistants.py index ec052f51..61e0465f 100644 --- a/backend/app/models/assistants.py +++ b/backend/app/models/assistants.py @@ -45,8 +45,12 @@ class Assistant(AssistantBase, table=True): class AssistantCreate(BaseModel): - name: str = PydanticField(description="Name of the assistant") - instructions: str = PydanticField(description="Instructions for the assistant") + name: str = PydanticField( + description="Name of the assistant", min_length=3, max_length=30 + ) + instructions: str = PydanticField( + description="Instructions for the assistant", min_length=3 + ) model: str = PydanticField( default="gpt-4o", description="Model name for the assistant" ) @@ -74,8 +78,12 @@ def validate_openai_model(cls, v): class AssistantUpdate(BaseModel): - name: str | None = None - instructions: str | None = None + name: str = PydanticField( + default=None, description="Name of the assistant", min_length=3, max_length=30 + ) + instructions: str | None = PydanticField( + default=None, min_length=3, description="Instructions for the assistant" + ) model: str | None = None vector_store_ids_add: list[str] | None = None vector_store_ids_remove: list[str] | None = None From 918d08cd1fdeba8c73ff6c5436d442ebfc08265f Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Tue, 22 Jul 2025 13:51:31 +0530 Subject: [PATCH 11/17] remove model validation --- backend/app/models/assistants.py | 34 +++++++++++--------------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/backend/app/models/assistants.py b/backend/app/models/assistants.py index 61e0465f..5e5c6065 100644 --- a/backend/app/models/assistants.py +++ b/backend/app/models/assistants.py @@ -1,16 +1,13 @@ from datetime import datetime from typing import List, Optional -from pydantic import BaseModel, Field as PydanticField, field_validator +from pydantic import BaseModel, Field as PydanticField from sqlalchemy import Column, String, Text from sqlalchemy.dialects.postgresql import ARRAY from sqlmodel import Field, Relationship, SQLModel - from app.core.util import now -ALLOWED_OPENAI_MODELS = {"gpt-3.5-turbo", "gpt-4", "gpt-4o"} - class AssistantBase(SQLModel): assistant_id: str = Field(index=True, unique=True) @@ -52,7 +49,10 @@ class AssistantCreate(BaseModel): description="Instructions for the assistant", min_length=3 ) model: str = PydanticField( - default="gpt-4o", description="Model name for the assistant" + default="gpt-4o", + description="Model name for the assistant", + min_length=1, + max_length=30, ) vector_store_ids: list[str] = PydanticField( default_factory=list, @@ -68,23 +68,19 @@ class AssistantCreate(BaseModel): description="Maximum number of results (must be between 1 and 100)", ) - @field_validator("model") - def validate_openai_model(cls, v): - if v not in ALLOWED_OPENAI_MODELS: - raise ValueError( - f"Model '{v}' is not a supported OpenAI model. Choose from: {', '.join(ALLOWED_OPENAI_MODELS)}" - ) - return v - class AssistantUpdate(BaseModel): name: str = PydanticField( default=None, description="Name of the assistant", min_length=3, max_length=30 ) instructions: str | None = PydanticField( - default=None, min_length=3, description="Instructions for the assistant" + default=None, + description="Instructions for the assistant", + min_length=3, + ) + model: str | None = PydanticField( + default=None, description="Name of the model", min_length=1, max_length=30 ) - model: str | None = None vector_store_ids_add: list[str] | None = None vector_store_ids_remove: list[str] | None = None temperature: float | None = PydanticField( @@ -96,11 +92,3 @@ class AssistantUpdate(BaseModel): le=100, description="Maximum number of results (must be between 1 and 100)", ) - - @field_validator("model") - def validate_openai_model(cls, v): - if v not in ALLOWED_OPENAI_MODELS: - raise ValueError( - f"Model '{v}' is not a supported OpenAI model. Choose from: {', '.join(ALLOWED_OPENAI_MODELS)}" - ) - return v From c25278c0359b58fea859966af540d89b7fc4a332 Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Tue, 22 Jul 2025 14:26:29 +0530 Subject: [PATCH 12/17] improve logs --- backend/app/crud/assistants.py | 19 +++++++++++++------ backend/app/utils.py | 7 +++++++ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/backend/app/crud/assistants.py b/backend/app/crud/assistants.py index 2260c31c..a198e385 100644 --- a/backend/app/crud/assistants.py +++ b/backend/app/crud/assistants.py @@ -109,8 +109,8 @@ def sync_assistant( 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, @@ -118,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.", @@ -179,6 +182,9 @@ def create_assistant( 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 @@ -210,10 +216,11 @@ def update_assistant( remove_ids = set(assistant_update.vector_store_ids_remove or []) if conflicting_ids := add_ids & remove_ids: logger.error( - f"Conflicting vector store IDs in add/remove: {conflicting_ids} | project_id: {project_id}" + f"[update_assistant] Conflicting vector store IDs in add/remove: {conflicting_ids} | project_id: {project_id}" ) - raise ValueError( - f"Cannot add and remove the same vector store IDs: {conflicting_ids}" + raise HTTPException( + status_code=400, + detail=f"Conflicting vector store IDs in add/remove: {conflicting_ids}.", ) # Add new vector store IDs @@ -248,7 +255,7 @@ def delete_assistant( """ existing_assistant = get_assistant_by_id(session, assistant_id, project_id) if not existing_assistant: - logger.error( + 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.") 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)}", From 8e701e80ffebb4bb4a86073ed1b196bdd485771b Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Tue, 22 Jul 2025 15:25:49 +0530 Subject: [PATCH 13/17] fix conflicting vector store id test cases --- backend/app/tests/crud/test_assistants.py | 25 +++++++++++++++-------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/backend/app/tests/crud/test_assistants.py b/backend/app/tests/crud/test_assistants.py index 503ea154..322eed77 100644 --- a/backend/app/tests/crud/test_assistants.py +++ b/backend/app/tests/crud/test_assistants.py @@ -250,23 +250,30 @@ def test_update_assistant_invalid_vector_store( assert error_message in exc_info.value.detail def test_update_assistant_conflicting_vector_store_ids(self, db: Session): - """Should raise ValueError when vector store IDs are both added and removed""" + """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=["vs_1"], vector_store_ids_remove=["vs_1"] + vector_store_ids_add=[conflicting_id], + vector_store_ids_remove=[conflicting_id], ) client = OpenAI(api_key="test_key") - with pytest.raises(ValueError) as exc_info: + + with pytest.raises(HTTPException) as exc_info: update_assistant( - db, - client, - assistant.assistant_id, - assistant.project_id, - assistant_update, + session=db, + openai_client=client, + assistant_id=assistant.assistant_id, + project_id=assistant.project_id, + assistant_update=assistant_update, ) - assert "Cannot add and remove the same vector store IDs" in str(exc_info.value) + 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( From 6c45f3a958cdb9e3588f3c63c1cefed65de18d03 Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Tue, 22 Jul 2025 17:46:32 +0530 Subject: [PATCH 14/17] use sql model instead of pydantic --- backend/app/models/assistants.py | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/backend/app/models/assistants.py b/backend/app/models/assistants.py index 5e5c6065..38fe6f77 100644 --- a/backend/app/models/assistants.py +++ b/backend/app/models/assistants.py @@ -1,7 +1,6 @@ from datetime import datetime from typing import List, Optional -from pydantic import BaseModel, Field as PydanticField from sqlalchemy import Column, String, Text from sqlalchemy.dialects.postgresql import ARRAY from sqlmodel import Field, Relationship, SQLModel @@ -41,27 +40,27 @@ class Assistant(AssistantBase, table=True): organization: "Organization" = Relationship(back_populates="assistants") -class AssistantCreate(BaseModel): - name: str = PydanticField( - description="Name of the assistant", min_length=3, max_length=30 +class AssistantCreate(SQLModel): + name: str = Field( + description="Name of the assistant", min_length=3, max_length=50 ) - instructions: str = PydanticField( + instructions: str = Field( description="Instructions for the assistant", min_length=3 ) - model: str = PydanticField( + model: str = Field( default="gpt-4o", description="Model name for the assistant", min_length=1, - max_length=30, + max_length=50, ) - vector_store_ids: list[str] = PydanticField( + vector_store_ids: list[str] = Field( default_factory=list, description="List of Vector Store IDs that exist in OpenAI.", ) - temperature: float | None = PydanticField( + temperature: float = Field( default=0.1, ge=0, le=2, description="Sampling temperature between 0 and 2" ) - max_num_results: int | None = PydanticField( + max_num_results: int = Field( default=20, ge=1, le=100, @@ -69,24 +68,24 @@ class AssistantCreate(BaseModel): ) -class AssistantUpdate(BaseModel): - name: str = PydanticField( +class AssistantUpdate(SQLModel): + name: str | None = Field( default=None, description="Name of the assistant", min_length=3, max_length=30 ) - instructions: str | None = PydanticField( + instructions: str | None = Field( default=None, description="Instructions for the assistant", min_length=3, ) - model: str | None = PydanticField( + model: str | None = Field( default=None, description="Name of the model", min_length=1, max_length=30 ) vector_store_ids_add: list[str] | None = None vector_store_ids_remove: list[str] | None = None - temperature: float | None = PydanticField( + temperature: float | None = Field( default=None, ge=0, le=2, description="Sampling temperature between 0 and 2" ) - max_num_results: int | None = PydanticField( + max_num_results: int | None = Field( default=None, ge=1, le=100, From 7819a325a8c3ffccb388db87380e4578b7e6621c Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Tue, 22 Jul 2025 18:05:08 +0530 Subject: [PATCH 15/17] change the max length for assistant update --- backend/app/models/assistants.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/backend/app/models/assistants.py b/backend/app/models/assistants.py index 38fe6f77..8c8f0239 100644 --- a/backend/app/models/assistants.py +++ b/backend/app/models/assistants.py @@ -45,7 +45,7 @@ class AssistantCreate(SQLModel): description="Name of the assistant", min_length=3, max_length=50 ) instructions: str = Field( - description="Instructions for the assistant", min_length=3 + description="Instructions for the assistant", min_length=10 ) model: str = Field( default="gpt-4o", @@ -70,15 +70,15 @@ class AssistantCreate(SQLModel): class AssistantUpdate(SQLModel): name: str | None = Field( - default=None, description="Name of the assistant", min_length=3, max_length=30 + 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=3, + min_length=10, ) model: str | None = Field( - default=None, description="Name of the model", min_length=1, max_length=30 + 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 From 6b4f695c97a90070dfe752f7dcc31efe762f72fb Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Tue, 22 Jul 2025 18:07:18 +0530 Subject: [PATCH 16/17] refactor code --- backend/app/crud/assistants.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/app/crud/assistants.py b/backend/app/crud/assistants.py index a198e385..244b1bb1 100644 --- a/backend/app/crud/assistants.py +++ b/backend/app/crud/assistants.py @@ -1,6 +1,6 @@ import logging -from typing import Optional, Set +from typing import Optional import uuid import openai @@ -174,7 +174,7 @@ def create_assistant( verify_vector_store_ids_exist(openai_client, assistant.vector_store_ids) assistant = Assistant( - assistant_id=uuid.uuid4(), + assistant_id=str(uuid.uuid4()), **assistant.model_dump(exclude_unset=True), project_id=project_id, organization_id=organization_id, @@ -209,7 +209,7 @@ def update_assistant( for field, value in update_fields.items(): setattr(existing_assistant, field, value) - current_vector_stores: Set[str] = set(existing_assistant.vector_store_ids or []) + 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 []) From b8a621bd3830a208211eedbb12388a5d8d877464 Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Wed, 23 Jul 2025 11:07:23 +0530 Subject: [PATCH 17/17] code refactor --- ...26b37_add_is_deleted_column_in_assistant_table.py | 4 ---- backend/app/crud/assistants.py | 12 ++++++------ backend/app/models/assistants.py | 8 +++----- backend/app/tests/api/routes/test_assistants.py | 2 +- 4 files changed, 10 insertions(+), 16 deletions(-) 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 index a7992fbf..5cea5342 100644 --- 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 @@ -17,18 +17,14 @@ def upgrade(): - # ### commands auto generated by Alembic - please adjust! ### 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) ) - # ### end Alembic commands ### def downgrade(): - # ### commands auto generated by Alembic - please adjust! ### op.drop_column("openai_assistant", "deleted_at") op.drop_column("openai_assistant", "is_deleted") - # ### end Alembic commands ### diff --git a/backend/app/crud/assistants.py b/backend/app/crud/assistants.py index 244b1bb1..6a8e8e5b 100644 --- a/backend/app/crud/assistants.py +++ b/backend/app/crud/assistants.py @@ -79,20 +79,20 @@ def verify_vector_store_ids_exist( """ Raises HTTPException if any of the vector_store_ids do not exist in OpenAI. """ - for vs_id in vector_store_ids: + for vector_store_id in vector_store_ids: try: - openai_client.vector_stores.retrieve(vs_id) + openai_client.vector_stores.retrieve(vector_store_id) except openai.NotFoundError: - logger.error(f"Vector store ID {vs_id} not found in OpenAI.") + logger.error(f"Vector store ID {vector_store_id} not found in OpenAI.") raise HTTPException( status_code=400, - detail=f"Vector store ID {vs_id} not found in OpenAI.", + 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 {vs_id}: {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 {vs_id}: {str(e)}", + detail=f"Error verifying vector store ID {vector_store_id}: {str(e)}", ) diff --git a/backend/app/models/assistants.py b/backend/app/models/assistants.py index 8c8f0239..b97e25b1 100644 --- a/backend/app/models/assistants.py +++ b/backend/app/models/assistants.py @@ -41,9 +41,7 @@ class Assistant(AssistantBase, table=True): class AssistantCreate(SQLModel): - name: str = Field( - description="Name of the assistant", min_length=3, max_length=50 - ) + 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 ) @@ -58,7 +56,7 @@ class AssistantCreate(SQLModel): description="List of Vector Store IDs that exist in OpenAI.", ) temperature: float = Field( - default=0.1, ge=0, le=2, description="Sampling temperature between 0 and 2" + default=0.1, gt=0, le=2, description="Sampling temperature between 0 and 2" ) max_num_results: int = Field( default=20, @@ -83,7 +81,7 @@ class AssistantUpdate(SQLModel): vector_store_ids_add: list[str] | None = None vector_store_ids_remove: list[str] | None = None temperature: float | None = Field( - default=None, ge=0, le=2, description="Sampling temperature between 0 and 2" + default=None, gt=0, le=2, description="Sampling temperature between 0 and 2" ) max_num_results: int | None = Field( default=None, diff --git a/backend/app/tests/api/routes/test_assistants.py b/backend/app/tests/api/routes/test_assistants.py index 47da4b2f..8755889d 100644 --- a/backend/app/tests/api/routes/test_assistants.py +++ b/backend/app/tests/api/routes/test_assistants.py @@ -13,7 +13,7 @@ def assistant_create_payload(): return { "name": "Test Assistant", - "instructions": "These are test instructions.", + "instructions": "This is a test instruction.", "model": "gpt-4o", "vector_store_ids": ["vs_test_1", "vs_test_2"], "temperature": 0.5,