From 77e5f86867f980d1c7b4af5850bafd8dc5a0a9f2 Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Thu, 10 Jul 2025 11:45:20 +0530 Subject: [PATCH 01/16] Convert vector store id to vector store ids --- ...change_vector_store_id_to_vector_store_.py | 38 +++++++++++++++++++ backend/app/api/routes/responses.py | 4 +- backend/app/models/assistants.py | 7 +++- 3 files changed, 46 insertions(+), 3 deletions(-) create mode 100644 backend/app/alembic/versions/f2589428c1d0_change_vector_store_id_to_vector_store_.py diff --git a/backend/app/alembic/versions/f2589428c1d0_change_vector_store_id_to_vector_store_.py b/backend/app/alembic/versions/f2589428c1d0_change_vector_store_id_to_vector_store_.py new file mode 100644 index 00000000..7065dbec --- /dev/null +++ b/backend/app/alembic/versions/f2589428c1d0_change_vector_store_id_to_vector_store_.py @@ -0,0 +1,38 @@ +"""Change vector_store_id to vector_store_ids in openai_assistant table + +Revision ID: f2589428c1d0 +Revises: 3389c67fdcb4 +Create Date: 2025-07-10 11:18:21.223114 + +""" +from alembic import op +import sqlalchemy as sa +import sqlmodel.sql.sqltypes +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = 'f2589428c1d0' +down_revision = '3389c67fdcb4' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('openai_assistant', sa.Column('vector_store_ids', postgresql.ARRAY(sa.String()), nullable=True)) + + op.execute(""" + UPDATE openai_assistant + SET vector_store_ids = ARRAY[vector_store_id] + WHERE vector_store_id IS NOT NULL + """) + + op.drop_column('openai_assistant', 'vector_store_id') + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('openai_assistant', sa.Column('vector_store_id', sa.VARCHAR(length=255), autoincrement=False, nullable=False)) + op.drop_column('openai_assistant', 'vector_store_ids') + # ### end Alembic commands ### diff --git a/backend/app/api/routes/responses.py b/backend/app/api/routes/responses.py index 233cab35..50121fe0 100644 --- a/backend/app/api/routes/responses.py +++ b/backend/app/api/routes/responses.py @@ -125,11 +125,11 @@ def process_response( "input": [{"role": "user", "content": request.question}], } - if assistant.vector_store_id: + if assistant.vector_store_ids: params["tools"] = [ { "type": "file_search", - "vector_store_ids": [assistant.vector_store_id], + "vector_store_ids": assistant.vector_store_ids, "max_num_results": assistant.max_num_results, } ] diff --git a/backend/app/models/assistants.py b/backend/app/models/assistants.py index 4163297d..7076ef30 100644 --- a/backend/app/models/assistants.py +++ b/backend/app/models/assistants.py @@ -1,6 +1,8 @@ from datetime import datetime from typing import Optional, List from sqlmodel import Field, Relationship, SQLModel +from sqlalchemy import Column, String +from sqlalchemy.dialects.postgresql import ARRAY from app.core.util import now @@ -10,7 +12,10 @@ class AssistantBase(SQLModel): name: str instructions: str model: str - vector_store_id: str + vector_store_ids: List[str] = Field( + default_factory=list, + sa_column=Column(ARRAY(String)) + ) temperature: float = 0.1 max_num_results: int = 20 project_id: int = Field(foreign_key="project.id") From b9dc7579b95e78c57db0e335ad763a0f44de084a Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Thu, 10 Jul 2025 17:09:33 +0530 Subject: [PATCH 02/16] Add api to scrap data from openAI --- ...change_vector_store_id_to_vector_store_.py | 29 +++-- backend/app/api/main.py | 2 + backend/app/api/routes/assistant.py | 113 ++++++++++++++++++ backend/app/crud/__init__.py | 3 + backend/app/models/assistants.py | 3 +- 5 files changed, 140 insertions(+), 10 deletions(-) create mode 100644 backend/app/api/routes/assistant.py diff --git a/backend/app/alembic/versions/f2589428c1d0_change_vector_store_id_to_vector_store_.py b/backend/app/alembic/versions/f2589428c1d0_change_vector_store_id_to_vector_store_.py index 7065dbec..a03c343c 100644 --- a/backend/app/alembic/versions/f2589428c1d0_change_vector_store_id_to_vector_store_.py +++ b/backend/app/alembic/versions/f2589428c1d0_change_vector_store_id_to_vector_store_.py @@ -11,28 +11,41 @@ from sqlalchemy.dialects import postgresql # revision identifiers, used by Alembic. -revision = 'f2589428c1d0' -down_revision = '3389c67fdcb4' +revision = "f2589428c1d0" +down_revision = "3389c67fdcb4" branch_labels = None depends_on = None def upgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.add_column('openai_assistant', sa.Column('vector_store_ids', postgresql.ARRAY(sa.String()), nullable=True)) + op.add_column( + "openai_assistant", + sa.Column("vector_store_ids", postgresql.ARRAY(sa.String()), nullable=True), + ) - op.execute(""" + op.execute( + """ UPDATE openai_assistant SET vector_store_ids = ARRAY[vector_store_id] WHERE vector_store_id IS NOT NULL - """) + """ + ) - op.drop_column('openai_assistant', 'vector_store_id') + op.drop_column("openai_assistant", "vector_store_id") # ### end Alembic commands ### def downgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.add_column('openai_assistant', sa.Column('vector_store_id', sa.VARCHAR(length=255), autoincrement=False, nullable=False)) - op.drop_column('openai_assistant', 'vector_store_ids') + op.add_column( + "openai_assistant", + sa.Column( + "vector_store_id", + sa.VARCHAR(length=255), + autoincrement=False, + nullable=False, + ), + ) + op.drop_column("openai_assistant", "vector_store_ids") # ### end Alembic commands ### diff --git a/backend/app/api/main.py b/backend/app/api/main.py index e6a74971..5bff1f28 100644 --- a/backend/app/api/main.py +++ b/backend/app/api/main.py @@ -15,11 +15,13 @@ utils, onboarding, credentials, + assistant, ) from app.core.config import settings api_router = APIRouter() api_router.include_router(api_keys.router) +api_router.include_router(assistant.router) api_router.include_router(collections.router) api_router.include_router(credentials.router) api_router.include_router(documents.router) diff --git a/backend/app/api/routes/assistant.py b/backend/app/api/routes/assistant.py new file mode 100644 index 00000000..e12c5019 --- /dev/null +++ b/backend/app/api/routes/assistant.py @@ -0,0 +1,113 @@ +import logging +from typing import Annotated + +from fastapi import APIRouter, Depends, HTTPException, Path, status +from sqlmodel import Session +import openai +from openai import OpenAI + +from app.api.deps import get_db, get_current_user_org_project +from app.crud import get_assistant_by_id, get_provider_credential +from app.models import Assistant, UserProjectOrg +from app.utils import APIResponse, mask_string + +logger = logging.getLogger(__name__) +router = APIRouter(prefix="/assistant", tags=["Assistants"]) + + +def handle_openai_error(e: openai.OpenAIError) -> str: + """Extract error message from OpenAI error.""" + if isinstance(e.body, dict) and "message" in e.body: + return e.body["message"] + return str(e) + + +@router.post("/{assistant_id}/ingest", response_model=APIResponse[Assistant], status_code=status.HTTP_201_CREATED) +def ingest_assistant( + assistant_id: Annotated[str, Path(description="The ID of the assistant to ingest")], + session: Session = Depends(get_db), + current_user: UserProjectOrg = Depends(get_current_user_org_project), +): + """ + Ingest a assistant from OpenAI and store it in the database. + """ + + existing_assistant = get_assistant_by_id(session, assistant_id, current_user.organization_id) + if existing_assistant: + logger.info(f"Assistant with ID {assistant_id} already exists in the database.") + metadata = {"message": f"Assistant with ID {assistant_id} already exists."} + return APIResponse( + success=True, + metadata= metadata, + data=existing_assistant + ) + + + credentials = get_provider_credential( + session=session, + org_id=current_user.organization_id, + provider="openai", + project_id=current_user.project_id, + ) + + if not credentials or "api_key" not in credentials: + logger.error( + f"OpenAI API key not configured for org_id={current_user.organization_id}, project_id={current_user.project_id}" + ) + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="OpenAI API key not configured for this organization or project.", + ) + + client = OpenAI(api_key=credentials["api_key"]) + + try: + assistant = client.beta.assistants.retrieve(assistant_id=assistant_id) + except openai.OpenAIError as e: + error_msg = handle_openai_error(e) + logger.error(f"OpenAI API error while retrieving assistant {mask_string(assistant_id)}: {error_msg}") + raise HTTPException(status_code=400, detail=f"OpenAI API error: {error_msg}") + + vector_store_ids = [] + if assistant.tool_resources and hasattr(assistant.tool_resources, "file_search"): + file_search = assistant.tool_resources.file_search + if file_search and hasattr(file_search, "vector_store_ids"): + vector_store_ids = file_search.vector_store_ids or [] + + max_num_results = 20 + for tool in assistant.tools or []: + if tool.type == "file_search": + file_search = getattr(tool, "file_search", None) + if file_search and hasattr(file_search, "max_num_results"): + max_num_results = file_search.max_num_results + break + + if not assistant.instructions: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Assistant has no instruction.", + ) + + db_assistant = Assistant( + assistant_id=assistant.id, + name=assistant.name or assistant.id, + instructions=assistant.instructions, + model=assistant.model, + vector_store_ids=vector_store_ids, + temperature=assistant.temperature or 0.1, + max_num_results=max_num_results, + project_id=current_user.project_id, + organization_id=current_user.organization_id, + ) + + session.add(db_assistant) + session.commit() + session.refresh(db_assistant) + + logger.info(f"Successfully ingested assistant with ID {assistant_id}.") + return APIResponse( + success=True, + message=f"Assistant with ID {assistant_id} ingested successfully.", + data=db_assistant + ) + \ No newline at end of file diff --git a/backend/app/crud/__init__.py b/backend/app/crud/__init__.py index 6bf4d5da..9697e9e0 100644 --- a/backend/app/crud/__init__.py +++ b/backend/app/crud/__init__.py @@ -5,6 +5,9 @@ update_user, ) from .collection import CollectionCrud + +from .credentials import get_provider_credential + from .document import DocumentCrud from .document_collection import DocumentCollectionCrud diff --git a/backend/app/models/assistants.py b/backend/app/models/assistants.py index 7076ef30..5647455d 100644 --- a/backend/app/models/assistants.py +++ b/backend/app/models/assistants.py @@ -13,8 +13,7 @@ class AssistantBase(SQLModel): instructions: str model: str vector_store_ids: List[str] = Field( - default_factory=list, - sa_column=Column(ARRAY(String)) + default_factory=list, sa_column=Column(ARRAY(String)) ) temperature: float = 0.1 max_num_results: int = 20 From 88b4cf064ed97ca4f620f70a79ba832a4161c582 Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Thu, 10 Jul 2025 20:22:13 +0530 Subject: [PATCH 03/16] precommit --- backend/app/api/routes/assistant.py | 34 +++++++++++++++-------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/backend/app/api/routes/assistant.py b/backend/app/api/routes/assistant.py index e12c5019..cccead46 100644 --- a/backend/app/api/routes/assistant.py +++ b/backend/app/api/routes/assistant.py @@ -22,27 +22,28 @@ def handle_openai_error(e: openai.OpenAIError) -> str: return str(e) -@router.post("/{assistant_id}/ingest", response_model=APIResponse[Assistant], status_code=status.HTTP_201_CREATED) +@router.post( + "/{assistant_id}/ingest", + response_model=APIResponse[Assistant], + status_code=status.HTTP_201_CREATED, +) def ingest_assistant( assistant_id: Annotated[str, Path(description="The ID of the assistant to ingest")], session: Session = Depends(get_db), current_user: UserProjectOrg = Depends(get_current_user_org_project), ): """ - Ingest a assistant from OpenAI and store it in the database. + Ingest a assistant from OpenAI and store it in the platform. """ - existing_assistant = get_assistant_by_id(session, assistant_id, current_user.organization_id) + existing_assistant = get_assistant_by_id( + session, assistant_id, current_user.organization_id + ) if existing_assistant: logger.info(f"Assistant with ID {assistant_id} already exists in the database.") metadata = {"message": f"Assistant with ID {assistant_id} already exists."} - return APIResponse( - success=True, - metadata= metadata, - data=existing_assistant - ) - - + return APIResponse(success=True, metadata=metadata, data=existing_assistant) + credentials = get_provider_credential( session=session, org_id=current_user.organization_id, @@ -65,7 +66,9 @@ def ingest_assistant( assistant = client.beta.assistants.retrieve(assistant_id=assistant_id) except openai.OpenAIError as e: error_msg = handle_openai_error(e) - logger.error(f"OpenAI API error while retrieving assistant {mask_string(assistant_id)}: {error_msg}") + logger.error( + f"OpenAI API error while retrieving assistant {mask_string(assistant_id)}: {error_msg}" + ) raise HTTPException(status_code=400, detail=f"OpenAI API error: {error_msg}") vector_store_ids = [] @@ -81,13 +84,13 @@ def ingest_assistant( if file_search and hasattr(file_search, "max_num_results"): max_num_results = file_search.max_num_results break - + if not assistant.instructions: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail="Assistant has no instruction.", ) - + db_assistant = Assistant( assistant_id=assistant.id, name=assistant.name or assistant.id, @@ -104,10 +107,9 @@ def ingest_assistant( session.commit() session.refresh(db_assistant) - logger.info(f"Successfully ingested assistant with ID {assistant_id}.") + logger.info(f"Successfully ingested assistant with ID {mask_string(assistant_id)}.") return APIResponse( success=True, message=f"Assistant with ID {assistant_id} ingested successfully.", - data=db_assistant + data=db_assistant, ) - \ No newline at end of file From 2f35b4f3331f215427a85137b2496bf7fd219a00 Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Thu, 10 Jul 2025 20:32:32 +0530 Subject: [PATCH 04/16] fix test --- backend/app/tests/api/routes/test_responses.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/app/tests/api/routes/test_responses.py b/backend/app/tests/api/routes/test_responses.py index e24b7a4d..d12d5252 100644 --- a/backend/app/tests/api/routes/test_responses.py +++ b/backend/app/tests/api/routes/test_responses.py @@ -80,7 +80,7 @@ def test_responses_endpoint_without_vector_store( mock_assistant.model = "gpt-4" mock_assistant.instructions = "Test instructions" mock_assistant.temperature = 0.1 - mock_assistant.vector_store_id = None # No vector store configured + mock_assistant.vector_store_ids = [] # No vector store configured mock_get_assistant.return_value = mock_assistant # Setup mock OpenAI client From 7cb0a9dcf5c881ee38b9c988125f1290d09b1d17 Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Sat, 12 Jul 2025 18:54:40 +0530 Subject: [PATCH 05/16] Move crud operation from routes to crud --- backend/app/api/routes/assistant.py | 101 ++++++---------------------- backend/app/crud/__init__.py | 6 +- backend/app/crud/assistants.py | 97 +++++++++++++++++++++++++- backend/app/utils.py | 9 +++ 4 files changed, 128 insertions(+), 85 deletions(-) diff --git a/backend/app/api/routes/assistant.py b/backend/app/api/routes/assistant.py index cccead46..e00a0cbe 100644 --- a/backend/app/api/routes/assistant.py +++ b/backend/app/api/routes/assistant.py @@ -1,115 +1,54 @@ -import logging from typing import Annotated from fastapi import APIRouter, Depends, HTTPException, Path, status from sqlmodel import Session -import openai -from openai import OpenAI from app.api.deps import get_db, get_current_user_org_project -from app.crud import get_assistant_by_id, get_provider_credential -from app.models import Assistant, UserProjectOrg -from app.utils import APIResponse, mask_string +from app.core.util import configure_openai +from app.crud import ( + fetch_assistant_from_openai, + get_provider_credential, + insert_assistant, +) +from app.models import UserProjectOrg +from app.utils import APIResponse -logger = logging.getLogger(__name__) router = APIRouter(prefix="/assistant", tags=["Assistants"]) -def handle_openai_error(e: openai.OpenAIError) -> str: - """Extract error message from OpenAI error.""" - if isinstance(e.body, dict) and "message" in e.body: - return e.body["message"] - return str(e) - - @router.post( "/{assistant_id}/ingest", - response_model=APIResponse[Assistant], + response_model=APIResponse, status_code=status.HTTP_201_CREATED, ) -def ingest_assistant( +def ingest_assistant_route( assistant_id: Annotated[str, Path(description="The ID of the assistant to ingest")], session: Session = Depends(get_db), current_user: UserProjectOrg = Depends(get_current_user_org_project), ): """ - Ingest a assistant from OpenAI and store it in the platform. + Ingest an assistant from OpenAI and store it in the platform. """ - - existing_assistant = get_assistant_by_id( - session, assistant_id, current_user.organization_id - ) - if existing_assistant: - logger.info(f"Assistant with ID {assistant_id} already exists in the database.") - metadata = {"message": f"Assistant with ID {assistant_id} already exists."} - return APIResponse(success=True, metadata=metadata, data=existing_assistant) - credentials = get_provider_credential( session=session, org_id=current_user.organization_id, provider="openai", project_id=current_user.project_id, ) + client, success = configure_openai(credentials) - if not credentials or "api_key" not in credentials: - logger.error( - f"OpenAI API key not configured for org_id={current_user.organization_id}, project_id={current_user.project_id}" - ) + if not success: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail="OpenAI API key not configured for this organization or project.", + detail="OpenAI not configured for this organization.", ) - client = OpenAI(api_key=credentials["api_key"]) - - try: - assistant = client.beta.assistants.retrieve(assistant_id=assistant_id) - except openai.OpenAIError as e: - error_msg = handle_openai_error(e) - logger.error( - f"OpenAI API error while retrieving assistant {mask_string(assistant_id)}: {error_msg}" - ) - raise HTTPException(status_code=400, detail=f"OpenAI API error: {error_msg}") - - vector_store_ids = [] - if assistant.tool_resources and hasattr(assistant.tool_resources, "file_search"): - file_search = assistant.tool_resources.file_search - if file_search and hasattr(file_search, "vector_store_ids"): - vector_store_ids = file_search.vector_store_ids or [] - - max_num_results = 20 - for tool in assistant.tools or []: - if tool.type == "file_search": - file_search = getattr(tool, "file_search", None) - if file_search and hasattr(file_search, "max_num_results"): - max_num_results = file_search.max_num_results - break - - if not assistant.instructions: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail="Assistant has no instruction.", - ) - - db_assistant = Assistant( - assistant_id=assistant.id, - name=assistant.name or assistant.id, - instructions=assistant.instructions, - model=assistant.model, - vector_store_ids=vector_store_ids, - temperature=assistant.temperature or 0.1, - max_num_results=max_num_results, - project_id=current_user.project_id, + openai_assistant = fetch_assistant_from_openai(assistant_id, client) + assistant = insert_assistant( + session=session, organization_id=current_user.organization_id, + project_id=current_user.project_id, + openai_assistant=openai_assistant, ) - session.add(db_assistant) - session.commit() - session.refresh(db_assistant) - - logger.info(f"Successfully ingested assistant with ID {mask_string(assistant_id)}.") - return APIResponse( - success=True, - message=f"Assistant with ID {assistant_id} ingested successfully.", - data=db_assistant, - ) + return APIResponse.success_response(assistant) diff --git a/backend/app/crud/__init__.py b/backend/app/crud/__init__.py index 9697e9e0..4f466fcf 100644 --- a/backend/app/crud/__init__.py +++ b/backend/app/crud/__init__.py @@ -44,4 +44,8 @@ from .thread_results import upsert_thread_result, get_thread_result -from .assistants import get_assistant_by_id +from .assistants import ( + get_assistant_by_id, + fetch_assistant_from_openai, + insert_assistant, +) diff --git a/backend/app/crud/assistants.py b/backend/app/crud/assistants.py index 5025c617..ee6da3bd 100644 --- a/backend/app/crud/assistants.py +++ b/backend/app/crud/assistants.py @@ -1,8 +1,17 @@ -from typing import Optional, List, Tuple -from sqlmodel import Session, select, and_ +import logging + +from typing import Optional + +import openai +from fastapi import HTTPException +from openai import OpenAI +from openai.types.beta import Assistant as OpenAIAssistant +from sqlmodel import Session, and_, select -from app.core.util import now from app.models import Assistant +from app.utils import APIResponse, handle_openai_error, mask_string + +logger = logging.getLogger(__name__) def get_assistant_by_id( @@ -16,3 +25,85 @@ def get_assistant_by_id( ) ) return session.exec(statement).first() + + +def fetch_assistant_from_openai(assistant_id: str, client: OpenAI) -> OpenAIAssistant: + """ + Fetch an assistant from OpenAI. + Returns OpenAI Assistant model. + """ + + try: + assistant = client.beta.assistants.retrieve(assistant_id=assistant_id) + return assistant + except openai.OpenAIError as e: + error_msg = handle_openai_error(e) + logger.error( + f"[get_assistant_by_id] OpenAI API error while retrieving assistant {mask_string(assistant_id)}: {error_msg}" + ) + raise HTTPException(status_code=400, detail=f"OpenAI API error: {error_msg}") + + +def insert_assistant( + session: Session, + organization_id: int, + project_id: int, + openai_assistant: OpenAIAssistant, +) -> APIResponse[Assistant]: + """ + Insert an assistant into the database by converting OpenAI Assistant to local Assistant model. + """ + assistant_id = openai_assistant.id + + existing_assistant = get_assistant_by_id(session, assistant_id, organization_id) + if existing_assistant: + logger.info( + f"[insert_assistant] Assistant with ID {assistant_id} already exists in the database." + ) + raise HTTPException( + status_code=409, + detail=f"Assistant with ID {assistant_id} already exists.", + ) + + if not openai_assistant.instructions: + raise HTTPException( + status_code=400, + detail="Assistant has no instruction.", + ) + + vector_store_ids = [] + if openai_assistant.tool_resources and hasattr( + openai_assistant.tool_resources, "file_search" + ): + file_search = openai_assistant.tool_resources.file_search + if file_search and hasattr(file_search, "vector_store_ids"): + vector_store_ids = file_search.vector_store_ids or [] + + max_num_results = 20 + for tool in openai_assistant.tools or []: + if tool.type == "file_search": + file_search = getattr(tool, "file_search", None) + if file_search and hasattr(file_search, "max_num_results"): + max_num_results = file_search.max_num_results + break + + db_assistant = Assistant( + assistant_id=openai_assistant.id, + name=openai_assistant.name or openai_assistant.id, + instructions=openai_assistant.instructions, + model=openai_assistant.model, + vector_store_ids=vector_store_ids, + temperature=openai_assistant.temperature or 0.1, + max_num_results=max_num_results, + project_id=project_id, + organization_id=organization_id, + ) + + session.add(db_assistant) + session.commit() + session.refresh(db_assistant) + + logger.info( + f"[insert_assistant] Successfully ingested assistant with ID {mask_string(assistant_id)}." + ) + return db_assistant diff --git a/backend/app/utils.py b/backend/app/utils.py index b7f7f793..bb722d03 100644 --- a/backend/app/utils.py +++ b/backend/app/utils.py @@ -10,6 +10,8 @@ from jinja2 import Template from jwt.exceptions import InvalidTokenError +import openai + from app.core import security from app.core.config import settings @@ -163,6 +165,13 @@ def mask_string(value: str, mask_char: str = "*") -> str: return value[:start] + (mask_char * num_mask) + value[end:] +def handle_openai_error(e: openai.OpenAIError) -> str: + """Extract error message from OpenAI error.""" + if isinstance(e.body, dict) and "message" in e.body: + return e.body["message"] + return str(e) + + @ft.singledispatch def load_description(filename: Path) -> str: if not filename.exists(): From 7e86924d8c4f0e9da581ef45b471b3a03a40cc79 Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Sun, 13 Jul 2025 15:20:37 +0530 Subject: [PATCH 06/16] fix seed to use vector_store_ids --- backend/app/seed_data/seed_data.json | 27 +++++++++++++++++++++++---- backend/app/seed_data/seed_data.py | 4 ++-- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/backend/app/seed_data/seed_data.json b/backend/app/seed_data/seed_data.json index 8fb40af5..0ad39f5c 100644 --- a/backend/app/seed_data/seed_data.json +++ b/backend/app/seed_data/seed_data.json @@ -57,23 +57,42 @@ { "is_active": true, "provider": "openai", - "credential": "{\"openai\": {\"api_key\": \"sk-proj-YxK21qI3i5SCxN\"}}", + "credential": "{\"api_key\": \"sk-proj-GlificI3i5SCxN\"}", "project_name": "Glific", "organization_name": "Project Tech4dev", "deleted_at": null + }, + { + "is_active": true, + "provider": "openai", + "credential": "{\"api_key\": \"sk-proj-DalgoI3i5SCxN\"}", + "project_name": "Dalgo", + "organization_name": "Project Tech4dev", + "deleted_at": null } ], "assistants": [ { - "assistant_id": "assistant_123", - "name": "Test Assistant", + "assistant_id": "assistant_glific", + "name": "Test Assistant Glific", "instructions": "Test instructions", "model": "gpt-4o", - "vector_store_id": "vs_123", + "vector_store_ids": ["vs_glific"], "temperature": 0.1, "max_num_results": 20, "project_name": "Glific", "organization_name": "Project Tech4dev" + }, + { + "assistant_id": "assistant_dalgo", + "name": "Test Assistant Dalgo", + "instructions": "Test instructions", + "model": "gpt-4o", + "vector_store_ids": ["vs_dalgo"], + "temperature": 0.1, + "max_num_results": 20, + "project_name": "Dalgo", + "organization_name": "Project Tech4dev" } ] } diff --git a/backend/app/seed_data/seed_data.py b/backend/app/seed_data/seed_data.py index 059c0794..f8a1c2ea 100644 --- a/backend/app/seed_data/seed_data.py +++ b/backend/app/seed_data/seed_data.py @@ -57,7 +57,7 @@ class AssistantData(BaseModel): name: str instructions: str model: str - vector_store_id: str + vector_store_ids: list[str] temperature: float max_num_results: int project_name: str @@ -261,7 +261,7 @@ def create_assistant(session: Session, assistant_data_raw: dict) -> Assistant: name=assistant_data.name, instructions=assistant_data.instructions, model=assistant_data.model, - vector_store_id=assistant_data.vector_store_id, + vector_store_ids=assistant_data.vector_store_ids, temperature=assistant_data.temperature, max_num_results=assistant_data.max_num_results, organization_id=organization.id, From a4072e1e4b14e36e8c69ed25201f24575b14eaca Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Sun, 13 Jul 2025 15:21:17 +0530 Subject: [PATCH 07/16] fix logs --- backend/app/crud/assistants.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/app/crud/assistants.py b/backend/app/crud/assistants.py index ee6da3bd..3c8daa82 100644 --- a/backend/app/crud/assistants.py +++ b/backend/app/crud/assistants.py @@ -39,7 +39,7 @@ def fetch_assistant_from_openai(assistant_id: str, client: OpenAI) -> OpenAIAssi except openai.OpenAIError as e: error_msg = handle_openai_error(e) logger.error( - f"[get_assistant_by_id] OpenAI API error while retrieving assistant {mask_string(assistant_id)}: {error_msg}" + f"[fetch_assistant_from_openai] OpenAI API error while retrieving assistant {mask_string(assistant_id)}: {error_msg}" ) raise HTTPException(status_code=400, detail=f"OpenAI API error: {error_msg}") From 309a0595cea7f4c1d9e70e9b8aa4bf8c8f7e4dd5 Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Sun, 13 Jul 2025 17:07:19 +0530 Subject: [PATCH 08/16] Add unit tests for assistant endpoints --- .../app/tests/api/routes/test_assistants.py | 65 +++++++++++ backend/app/tests/crud/test_assistants.py | 109 ++++++++++++++++++ backend/app/tests/utils/openai.py | 42 +++++++ backend/app/tests/utils/utils.py | 31 ++++- 4 files changed, 243 insertions(+), 4 deletions(-) create mode 100644 backend/app/tests/api/routes/test_assistants.py create mode 100644 backend/app/tests/crud/test_assistants.py create mode 100644 backend/app/tests/utils/openai.py diff --git a/backend/app/tests/api/routes/test_assistants.py b/backend/app/tests/api/routes/test_assistants.py new file mode 100644 index 00000000..41e98ecf --- /dev/null +++ b/backend/app/tests/api/routes/test_assistants.py @@ -0,0 +1,65 @@ +import pytest +from fastapi.testclient import TestClient +from sqlmodel import Session +from unittest.mock import MagicMock, patch +from app.main import app +from app.tests.utils.utils import random_email +from app.core.security import get_password_hash +from app.tests.utils.openai import mock_openai_assistant + +client = TestClient(app) + + +@pytest.fixture +def normal_user_api_key_header(): + return {"X-API-KEY":"ApiKey Px8y47B6roJHin1lWLkR88eiDrFdXSJRZmFQazzai8j9"} + + +@patch("app.api.routes.assistant.fetch_assistant_from_openai") +def test_ingest_assistant_success( + mock_fetch_assistant, + db: Session, + normal_user_api_key_header: str, +): + """Test successful assistant ingestion from OpenAI.""" + + # Setup mock return value + + mock_assistant = mock_openai_assistant() + + mock_fetch_assistant.return_value = mock_assistant + + response = client.post( + f"/api/v1/assistant/{mock_assistant.id}/ingest", + headers=normal_user_api_key_header, + ) + + assert response.status_code == 201 + response_json = response.json() + assert response_json["success"] is True + assert response_json["data"]["assistant_id"] == mock_assistant.id + + +@patch("app.api.routes.assistant.configure_openai") +def test_ingest_assistant_openai_not_configured( + mock_configure_openai, + db: Session, + normal_user_api_key_header: dict, +): + """Test assistant ingestion failure when OpenAI is not configured.""" + + # Setup mock to return failure for OpenAI configuration + mock_configure_openai.return_value = (None, False) + + # Use a mock assistant ID + mock_assistant_id = "asst_123456789" + + response = client.post( + f"/api/v1/assistant/{mock_assistant_id}/ingest", + headers=normal_user_api_key_header, + ) + + assert response.status_code == 400 + response_json = response.json() + assert response_json["success"] is False + assert response_json["error"] == "OpenAI not configured for this organization." diff --git a/backend/app/tests/crud/test_assistants.py b/backend/app/tests/crud/test_assistants.py new file mode 100644 index 00000000..0c7b1401 --- /dev/null +++ b/backend/app/tests/crud/test_assistants.py @@ -0,0 +1,109 @@ +import pytest +from sqlmodel import Session +from fastapi import HTTPException + +from app.tests.utils.openai import mock_openai_assistant +from app.tests.utils.utils import get_project +from app.crud.assistants import insert_assistant + + +class TestAssistant: + def test_insert_assistant_success(self, db: Session): + project = get_project(db) + openai_assistant = mock_openai_assistant( + assistant_id="asst_success", + vector_store_ids=["vs_1", "vs_2"], + max_num_results=20 + ) + + result = insert_assistant(db, project.organization_id, project.id, openai_assistant) + + assert result.assistant_id == openai_assistant.id + assert result.project_id == project.id + assert result.organization_id == project.organization_id + assert result.name == openai_assistant.name + assert result.instructions == openai_assistant.instructions + assert result.model == openai_assistant.model + assert result.vector_store_ids == ["vs_1", "vs_2"] + assert result.temperature == openai_assistant.temperature + assert result.max_num_results == 20 + + def test_insert_assistant_already_exists(self, db: Session): + project = get_project(db) + openai_assistant = mock_openai_assistant( + assistant_id="asst_exists", + ) + + insert_assistant(db, project.organization_id, project.id, openai_assistant) + + with pytest.raises(HTTPException) as exc_info: + insert_assistant(db, project.organization_id, project.id, openai_assistant) + + assert exc_info.value.status_code == 409 + assert "already exists" in exc_info.value.detail + + def test_insert_assistant_no_instructions(self, db: Session): + project = get_project(db) + openai_assistant = mock_openai_assistant( + assistant_id="asst_no_instructions", + ) + openai_assistant.instructions = None + + with pytest.raises(HTTPException) as exc_info: + insert_assistant(db, project.organization_id, project.id, openai_assistant) + + assert exc_info.value.status_code == 400 + assert "no instruction" in exc_info.value.detail + + def test_insert_assistant_no_name(self, db: Session): + project = get_project(db) + openai_assistant = mock_openai_assistant( + assistant_id="asst_no_name", + ) + openai_assistant.name = None + + result = insert_assistant(db, project.organization_id, project.id, openai_assistant) + + assert result.name == openai_assistant.id + assert result.assistant_id == openai_assistant.id + assert result.project_id == project.id + + def test_insert_assistant_no_vector_stores(self, db: Session): + project = get_project(db) + openai_assistant = mock_openai_assistant( + assistant_id="asst_no_vectors", + vector_store_ids=None + ) + + result = insert_assistant(db, project.organization_id, project.id, openai_assistant) + + assert result.vector_store_ids == [] + assert result.assistant_id == openai_assistant.id + assert result.project_id == project.id + + + def test_insert_assistant_no_tools(self, db: Session): + project = get_project(db) + openai_assistant = mock_openai_assistant( + assistant_id="asst_no_tools" + ) + + openai_assistant.tool_resources = None + result = insert_assistant(db, project.organization_id, project.id, openai_assistant) + + assert result.vector_store_ids == [] # Default value + assert result.assistant_id == openai_assistant.id + assert result.project_id == project.id + + def test_insert_assistant_no_tool_resources(self, db: Session): + project = get_project(db) + openai_assistant = mock_openai_assistant( + assistant_id="asst_no_tool_resources", + ) + openai_assistant.tools = None + + result = insert_assistant(db, project.organization_id, project.id, openai_assistant) + + assert result.max_num_results == 20 + assert result.assistant_id == openai_assistant.id + assert result.project_id == project.id diff --git a/backend/app/tests/utils/openai.py b/backend/app/tests/utils/openai.py new file mode 100644 index 00000000..d5ff3a15 --- /dev/null +++ b/backend/app/tests/utils/openai.py @@ -0,0 +1,42 @@ +from typing import Optional +import time + +from openai.types.beta import Assistant as OpenAIAssistant +from openai.types.beta.assistant import ToolResources, ToolResourcesFileSearch +from openai.types.beta.assistant_tool import FileSearchTool +from openai.types.beta.file_search_tool import FileSearch + + +def mock_openai_assistant( + assistant_id: str = "assistant_mock", + vector_store_ids: Optional[list[str]] = ['vs_1', 'vs_2'], + max_num_results: int = 30, +) -> OpenAIAssistant: + + return OpenAIAssistant( + id=assistant_id, + created_at=int(time.time()), + description="Mock description", + instructions="Mock instructions", + metadata={}, + model="gpt-4o", + name="Mock Assistant", + object="assistant", + tools=[ + FileSearchTool( + type="file_search", + file_search=FileSearch( + max_num_results=max_num_results, + ), + ) + ], + temperature=1.0, + tool_resources=ToolResources( + code_interpreter=None, + file_search=ToolResourcesFileSearch( + vector_store_ids=vector_store_ids + ), + ), + top_p=1.0, + reasoning_effort=None, + ) diff --git a/backend/app/tests/utils/utils.py b/backend/app/tests/utils/utils.py index 9cba7aae..8093ffdf 100644 --- a/backend/app/tests/utils/utils.py +++ b/backend/app/tests/utils/utils.py @@ -1,17 +1,16 @@ import random import string from uuid import UUID +from typing import Optional, Type, TypeVar import pytest from fastapi.testclient import TestClient from sqlmodel import Session, select -from typing import Type, TypeVar from app.core.config import settings from app.crud.user import get_user_by_email -from app.models import APIKeyPublic -from app.crud import create_api_key, get_api_key_by_value -from uuid import uuid4 +from app.crud.api_key import get_api_key_by_value +from app.models import APIKeyPublic, Project T = TypeVar("T") @@ -60,6 +59,30 @@ def get_non_existent_id(session: Session, model: Type[T]) -> int: return (result or 0) + 1 +def get_project(session: Session, name: Optional[str] = None) -> Project: + """ + Retrieve an active project from the database. + + If a project name is provided, fetch the active project with that name. + If no name is provided, fetch any random project. + """ + if name: + statement = ( + select(Project) + .where(Project.name == name, Project.is_active == True) + .limit(1) + ) + else: + statement = select(Project).where(Project.is_active == True).limit(1) + + project = session.exec(statement).first() + + if not project: + raise ValueError("No active projects found") + + return project + + class SequentialUuidGenerator: def __init__(self, start=0): self.start = start From 77ac3f29262d6597bece1d3c33d93073df51fb80 Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Sun, 13 Jul 2025 17:17:06 +0530 Subject: [PATCH 09/16] precommit and code rafactoring --- backend/app/api/main.py | 4 +-- .../routes/{assistant.py => assistants.py} | 0 .../app/tests/api/routes/test_assistants.py | 16 ++++----- .../app/tests/api/routes/test_responses.py | 2 +- backend/app/tests/crud/test_assistants.py | 34 +++++++++++-------- backend/app/tests/utils/openai.py | 7 ++-- 6 files changed, 33 insertions(+), 30 deletions(-) rename backend/app/api/routes/{assistant.py => assistants.py} (100%) diff --git a/backend/app/api/main.py b/backend/app/api/main.py index 5bff1f28..7db3c3d5 100644 --- a/backend/app/api/main.py +++ b/backend/app/api/main.py @@ -2,6 +2,7 @@ from app.api.routes import ( api_keys, + assistants, collections, documents, login, @@ -15,13 +16,12 @@ utils, onboarding, credentials, - assistant, ) from app.core.config import settings api_router = APIRouter() api_router.include_router(api_keys.router) -api_router.include_router(assistant.router) +api_router.include_router(assistants.router) api_router.include_router(collections.router) api_router.include_router(credentials.router) api_router.include_router(documents.router) diff --git a/backend/app/api/routes/assistant.py b/backend/app/api/routes/assistants.py similarity index 100% rename from backend/app/api/routes/assistant.py rename to backend/app/api/routes/assistants.py diff --git a/backend/app/tests/api/routes/test_assistants.py b/backend/app/tests/api/routes/test_assistants.py index 41e98ecf..46430e9c 100644 --- a/backend/app/tests/api/routes/test_assistants.py +++ b/backend/app/tests/api/routes/test_assistants.py @@ -12,10 +12,10 @@ @pytest.fixture def normal_user_api_key_header(): - return {"X-API-KEY":"ApiKey Px8y47B6roJHin1lWLkR88eiDrFdXSJRZmFQazzai8j9"} + return {"X-API-KEY": "ApiKey Px8y47B6roJHin1lWLkR88eiDrFdXSJRZmFQazzai8j9"} -@patch("app.api.routes.assistant.fetch_assistant_from_openai") +@patch("app.api.routes.assistants.fetch_assistant_from_openai") def test_ingest_assistant_success( mock_fetch_assistant, db: Session, @@ -38,27 +38,27 @@ 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.api.routes.assistant.configure_openai") + +@patch("app.api.routes.assistants.configure_openai") def test_ingest_assistant_openai_not_configured( mock_configure_openai, db: Session, normal_user_api_key_header: dict, ): """Test assistant ingestion failure when OpenAI is not configured.""" - + # Setup mock to return failure for OpenAI configuration mock_configure_openai.return_value = (None, False) - + # Use a mock assistant ID mock_assistant_id = "asst_123456789" - + response = client.post( f"/api/v1/assistant/{mock_assistant_id}/ingest", headers=normal_user_api_key_header, ) - + assert response.status_code == 400 response_json = response.json() assert response_json["success"] is False diff --git a/backend/app/tests/api/routes/test_responses.py b/backend/app/tests/api/routes/test_responses.py index d12d5252..cec03e26 100644 --- a/backend/app/tests/api/routes/test_responses.py +++ b/backend/app/tests/api/routes/test_responses.py @@ -49,7 +49,7 @@ def test_responses_endpoint_success( headers = {"X-API-KEY": original_api_key} request_data = { - "assistant_id": "assistant_123", + "assistant_id": "assistant_glific", "question": "What is Glific?", "callback_url": "http://example.com/callback", } diff --git a/backend/app/tests/crud/test_assistants.py b/backend/app/tests/crud/test_assistants.py index 0c7b1401..47457b84 100644 --- a/backend/app/tests/crud/test_assistants.py +++ b/backend/app/tests/crud/test_assistants.py @@ -13,10 +13,12 @@ def test_insert_assistant_success(self, db: Session): openai_assistant = mock_openai_assistant( assistant_id="asst_success", vector_store_ids=["vs_1", "vs_2"], - max_num_results=20 + max_num_results=20, ) - result = insert_assistant(db, project.organization_id, project.id, openai_assistant) + result = insert_assistant( + db, project.organization_id, project.id, openai_assistant + ) assert result.assistant_id == openai_assistant.id assert result.project_id == project.id @@ -38,7 +40,7 @@ def test_insert_assistant_already_exists(self, db: Session): with pytest.raises(HTTPException) as exc_info: insert_assistant(db, project.organization_id, project.id, openai_assistant) - + assert exc_info.value.status_code == 409 assert "already exists" in exc_info.value.detail @@ -51,7 +53,7 @@ def test_insert_assistant_no_instructions(self, db: Session): with pytest.raises(HTTPException) as exc_info: insert_assistant(db, project.organization_id, project.id, openai_assistant) - + assert exc_info.value.status_code == 400 assert "no instruction" in exc_info.value.detail @@ -62,7 +64,9 @@ def test_insert_assistant_no_name(self, db: Session): ) openai_assistant.name = None - result = insert_assistant(db, project.organization_id, project.id, openai_assistant) + result = insert_assistant( + db, project.organization_id, project.id, openai_assistant + ) assert result.name == openai_assistant.id assert result.assistant_id == openai_assistant.id @@ -71,25 +75,25 @@ def test_insert_assistant_no_name(self, db: Session): def test_insert_assistant_no_vector_stores(self, db: Session): project = get_project(db) openai_assistant = mock_openai_assistant( - assistant_id="asst_no_vectors", - vector_store_ids=None + assistant_id="asst_no_vectors", vector_store_ids=None ) - result = insert_assistant(db, project.organization_id, project.id, openai_assistant) + result = insert_assistant( + db, project.organization_id, project.id, openai_assistant + ) assert result.vector_store_ids == [] assert result.assistant_id == openai_assistant.id assert result.project_id == project.id - def test_insert_assistant_no_tools(self, db: Session): project = get_project(db) - openai_assistant = mock_openai_assistant( - assistant_id="asst_no_tools" - ) + openai_assistant = mock_openai_assistant(assistant_id="asst_no_tools") openai_assistant.tool_resources = None - result = insert_assistant(db, project.organization_id, project.id, openai_assistant) + result = insert_assistant( + db, project.organization_id, project.id, openai_assistant + ) assert result.vector_store_ids == [] # Default value assert result.assistant_id == openai_assistant.id @@ -102,7 +106,9 @@ def test_insert_assistant_no_tool_resources(self, db: Session): ) openai_assistant.tools = None - result = insert_assistant(db, project.organization_id, project.id, openai_assistant) + result = insert_assistant( + db, project.organization_id, project.id, openai_assistant + ) assert result.max_num_results == 20 assert result.assistant_id == openai_assistant.id diff --git a/backend/app/tests/utils/openai.py b/backend/app/tests/utils/openai.py index d5ff3a15..778d4804 100644 --- a/backend/app/tests/utils/openai.py +++ b/backend/app/tests/utils/openai.py @@ -9,10 +9,9 @@ def mock_openai_assistant( assistant_id: str = "assistant_mock", - vector_store_ids: Optional[list[str]] = ['vs_1', 'vs_2'], + vector_store_ids: Optional[list[str]] = ["vs_1", "vs_2"], max_num_results: int = 30, ) -> OpenAIAssistant: - return OpenAIAssistant( id=assistant_id, created_at=int(time.time()), @@ -33,9 +32,7 @@ def mock_openai_assistant( temperature=1.0, tool_resources=ToolResources( code_interpreter=None, - file_search=ToolResourcesFileSearch( - vector_store_ids=vector_store_ids - ), + file_search=ToolResourcesFileSearch(vector_store_ids=vector_store_ids), ), top_p=1.0, reasoning_effort=None, From a0203f6eeb0fb4777b1dd1906de8664ea5f31650 Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Sun, 13 Jul 2025 17:20:08 +0530 Subject: [PATCH 10/16] remove open ai error handler --- backend/app/crud/assistants.py | 7 +++---- backend/app/utils.py | 9 --------- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/backend/app/crud/assistants.py b/backend/app/crud/assistants.py index 3c8daa82..cf7c8dd5 100644 --- a/backend/app/crud/assistants.py +++ b/backend/app/crud/assistants.py @@ -9,7 +9,7 @@ from sqlmodel import Session, and_, select from app.models import Assistant -from app.utils import APIResponse, handle_openai_error, mask_string +from app.utils import APIResponse, mask_string logger = logging.getLogger(__name__) @@ -37,11 +37,10 @@ def fetch_assistant_from_openai(assistant_id: str, client: OpenAI) -> OpenAIAssi assistant = client.beta.assistants.retrieve(assistant_id=assistant_id) return assistant except openai.OpenAIError as e: - error_msg = handle_openai_error(e) logger.error( - f"[fetch_assistant_from_openai] OpenAI API error while retrieving assistant {mask_string(assistant_id)}: {error_msg}" + f"[fetch_assistant_from_openai] OpenAI API error while retrieving assistant {mask_string(assistant_id)}: {e}" ) - raise HTTPException(status_code=400, detail=f"OpenAI API error: {error_msg}") + raise HTTPException(status_code=400, detail=f"OpenAI API error: {e}") def insert_assistant( diff --git a/backend/app/utils.py b/backend/app/utils.py index bb722d03..b7f7f793 100644 --- a/backend/app/utils.py +++ b/backend/app/utils.py @@ -10,8 +10,6 @@ from jinja2 import Template from jwt.exceptions import InvalidTokenError -import openai - from app.core import security from app.core.config import settings @@ -165,13 +163,6 @@ def mask_string(value: str, mask_char: str = "*") -> str: return value[:start] + (mask_char * num_mask) + value[end:] -def handle_openai_error(e: openai.OpenAIError) -> str: - """Extract error message from OpenAI error.""" - if isinstance(e.body, dict) and "message" in e.body: - return e.body["message"] - return str(e) - - @ft.singledispatch def load_description(filename: Path) -> str: if not filename.exists(): From 233ab8281096fe1dda709319e7cb21a17fa4efaa Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Sun, 13 Jul 2025 17:24:56 +0530 Subject: [PATCH 11/16] refactor --- backend/app/crud/__init__.py | 3 +-- backend/app/crud/assistants.py | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/backend/app/crud/__init__.py b/backend/app/crud/__init__.py index 4f466fcf..d3dafe6b 100644 --- a/backend/app/crud/__init__.py +++ b/backend/app/crud/__init__.py @@ -6,8 +6,6 @@ ) from .collection import CollectionCrud -from .credentials import get_provider_credential - from .document import DocumentCrud from .document_collection import DocumentCollectionCrud @@ -38,6 +36,7 @@ set_creds_for_org, get_creds_by_org, get_key_by_org, + get_provider_credential, update_creds_for_org, remove_creds_for_org, ) diff --git a/backend/app/crud/assistants.py b/backend/app/crud/assistants.py index cf7c8dd5..6de2dd25 100644 --- a/backend/app/crud/assistants.py +++ b/backend/app/crud/assistants.py @@ -9,7 +9,7 @@ from sqlmodel import Session, and_, select from app.models import Assistant -from app.utils import APIResponse, mask_string +from app.utils import mask_string logger = logging.getLogger(__name__) @@ -48,7 +48,7 @@ def insert_assistant( organization_id: int, project_id: int, openai_assistant: OpenAIAssistant, -) -> APIResponse[Assistant]: +) -> Assistant: """ Insert an assistant into the database by converting OpenAI Assistant to local Assistant model. """ From 6dd98d7c98a16fc0a1d658f159b033027445e746 Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Mon, 14 Jul 2025 10:35:17 +0530 Subject: [PATCH 12/16] remove unused imports and rename migration file --- ...ge_vector_store_id_to_vector_store_ids.py} | 21 +++++++++++++++---- .../app/tests/api/routes/test_assistants.py | 7 +------ backend/app/tests/utils/utils.py | 8 +++---- 3 files changed, 22 insertions(+), 14 deletions(-) rename backend/app/alembic/versions/{f2589428c1d0_change_vector_store_id_to_vector_store_.py => f2589428c1d0_change_vector_store_id_to_vector_store_ids.py} (69%) diff --git a/backend/app/alembic/versions/f2589428c1d0_change_vector_store_id_to_vector_store_.py b/backend/app/alembic/versions/f2589428c1d0_change_vector_store_id_to_vector_store_ids.py similarity index 69% rename from backend/app/alembic/versions/f2589428c1d0_change_vector_store_id_to_vector_store_.py rename to backend/app/alembic/versions/f2589428c1d0_change_vector_store_id_to_vector_store_ids.py index a03c343c..b25bec47 100644 --- a/backend/app/alembic/versions/f2589428c1d0_change_vector_store_id_to_vector_store_.py +++ b/backend/app/alembic/versions/f2589428c1d0_change_vector_store_id_to_vector_store_ids.py @@ -7,7 +7,6 @@ """ from alembic import op import sqlalchemy as sa -import sqlmodel.sql.sqltypes from sqlalchemy.dialects import postgresql # revision identifiers, used by Alembic. @@ -37,15 +36,29 @@ def upgrade(): def downgrade(): - # ### commands auto generated by Alembic - please adjust! ### + # Add back the single vector_store_id column as nullable for safe data migration op.add_column( "openai_assistant", sa.Column( "vector_store_id", sa.VARCHAR(length=255), autoincrement=False, - nullable=False, + nullable=True, # Allow nulls temporarily for safe migration ), ) + + op.execute( + """ + UPDATE openai_assistant + SET vector_store_id = vector_store_ids[1] + WHERE vector_store_ids IS NOT NULL AND array_length(vector_store_ids, 1) > 0 + """ + ) + op.drop_column("openai_assistant", "vector_store_ids") - # ### end Alembic commands ### + + op.alter_column( + "openai_assistant", + "vector_store_id", + existing_type=sa.VARCHAR(length=255), + ) diff --git a/backend/app/tests/api/routes/test_assistants.py b/backend/app/tests/api/routes/test_assistants.py index 46430e9c..b3ca1d89 100644 --- a/backend/app/tests/api/routes/test_assistants.py +++ b/backend/app/tests/api/routes/test_assistants.py @@ -1,10 +1,7 @@ import pytest from fastapi.testclient import TestClient -from sqlmodel import Session -from unittest.mock import MagicMock, patch +from unittest.mock import patch from app.main import app -from app.tests.utils.utils import random_email -from app.core.security import get_password_hash from app.tests.utils.openai import mock_openai_assistant client = TestClient(app) @@ -18,7 +15,6 @@ def normal_user_api_key_header(): @patch("app.api.routes.assistants.fetch_assistant_from_openai") def test_ingest_assistant_success( mock_fetch_assistant, - db: Session, normal_user_api_key_header: str, ): """Test successful assistant ingestion from OpenAI.""" @@ -43,7 +39,6 @@ def test_ingest_assistant_success( @patch("app.api.routes.assistants.configure_openai") def test_ingest_assistant_openai_not_configured( mock_configure_openai, - db: Session, normal_user_api_key_header: dict, ): """Test assistant ingestion failure when OpenAI is not configured.""" diff --git a/backend/app/tests/utils/utils.py b/backend/app/tests/utils/utils.py index 8093ffdf..c898128a 100644 --- a/backend/app/tests/utils/utils.py +++ b/backend/app/tests/utils/utils.py @@ -1,7 +1,7 @@ import random import string from uuid import UUID -from typing import Optional, Type, TypeVar +from typing import Type, TypeVar import pytest from fastapi.testclient import TestClient @@ -59,7 +59,7 @@ def get_non_existent_id(session: Session, model: Type[T]) -> int: return (result or 0) + 1 -def get_project(session: Session, name: Optional[str] = None) -> Project: +def get_project(session: Session, name: str | None = None) -> Project: """ Retrieve an active project from the database. @@ -69,11 +69,11 @@ def get_project(session: Session, name: Optional[str] = None) -> Project: if name: statement = ( select(Project) - .where(Project.name == name, Project.is_active == True) + .where(Project.name == name, Project.is_active) .limit(1) ) else: - statement = select(Project).where(Project.is_active == True).limit(1) + statement = select(Project).where(Project.is_active).limit(1) project = session.exec(statement).first() From 63c3b2616fc7c5d0bfb76bd9ede29ba6994326fa Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Mon, 14 Jul 2025 10:42:52 +0530 Subject: [PATCH 13/16] precommit --- ...589428c1d0_change_vector_store_id_to_vector_store_ids.py | 6 ------ backend/app/tests/utils/utils.py | 4 +--- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/backend/app/alembic/versions/f2589428c1d0_change_vector_store_id_to_vector_store_ids.py b/backend/app/alembic/versions/f2589428c1d0_change_vector_store_id_to_vector_store_ids.py index b25bec47..f8c49fad 100644 --- a/backend/app/alembic/versions/f2589428c1d0_change_vector_store_id_to_vector_store_ids.py +++ b/backend/app/alembic/versions/f2589428c1d0_change_vector_store_id_to_vector_store_ids.py @@ -56,9 +56,3 @@ def downgrade(): ) op.drop_column("openai_assistant", "vector_store_ids") - - op.alter_column( - "openai_assistant", - "vector_store_id", - existing_type=sa.VARCHAR(length=255), - ) diff --git a/backend/app/tests/utils/utils.py b/backend/app/tests/utils/utils.py index c898128a..6e94599a 100644 --- a/backend/app/tests/utils/utils.py +++ b/backend/app/tests/utils/utils.py @@ -68,9 +68,7 @@ def get_project(session: Session, name: str | None = None) -> Project: """ if name: statement = ( - select(Project) - .where(Project.name == name, Project.is_active) - .limit(1) + select(Project).where(Project.name == name, Project.is_active).limit(1) ) else: statement = select(Project).where(Project.is_active).limit(1) From af6a40b5b72c6e48f0d1d1ad77fa5df49e767bb5 Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Wed, 16 Jul 2025 13:53:56 +0530 Subject: [PATCH 14/16] Code refactoring --- backend/app/api/routes/assistants.py | 27 +++++--------- backend/app/crud/__init__.py | 3 +- backend/app/crud/assistants.py | 9 +++-- .../app/tests/api/routes/test_assistants.py | 30 +--------------- backend/app/tests/crud/test_assistants.py | 32 ++++++++--------- backend/app/utils.py | 36 ++++++++++++++++--- 6 files changed, 65 insertions(+), 72 deletions(-) diff --git a/backend/app/api/routes/assistants.py b/backend/app/api/routes/assistants.py index e00a0cbe..35f66146 100644 --- a/backend/app/api/routes/assistants.py +++ b/backend/app/api/routes/assistants.py @@ -1,17 +1,15 @@ from typing import Annotated -from fastapi import APIRouter, Depends, HTTPException, Path, status +from fastapi import APIRouter, Depends, Path from sqlmodel import Session from app.api.deps import get_db, get_current_user_org_project -from app.core.util import configure_openai from app.crud import ( fetch_assistant_from_openai, - get_provider_credential, - insert_assistant, + sync_assistant, ) from app.models import UserProjectOrg -from app.utils import APIResponse +from app.utils import APIResponse, get_openai_client router = APIRouter(prefix="/assistant", tags=["Assistants"]) @@ -19,7 +17,7 @@ @router.post( "/{assistant_id}/ingest", response_model=APIResponse, - status_code=status.HTTP_201_CREATED, + status_code=201, ) def ingest_assistant_route( assistant_id: Annotated[str, Path(description="The ID of the assistant to ingest")], @@ -29,22 +27,13 @@ def ingest_assistant_route( """ Ingest an assistant from OpenAI and store it in the platform. """ - credentials = get_provider_credential( - session=session, - org_id=current_user.organization_id, - provider="openai", - project_id=current_user.project_id, - ) - client, success = configure_openai(credentials) - if not success: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail="OpenAI not configured for this organization.", - ) + client = get_openai_client( + session, current_user.organization_id, current_user.project_id + ) openai_assistant = fetch_assistant_from_openai(assistant_id, client) - assistant = insert_assistant( + assistant = sync_assistant( session=session, organization_id=current_user.organization_id, project_id=current_user.project_id, diff --git a/backend/app/crud/__init__.py b/backend/app/crud/__init__.py index 338ac702..6ec996c2 100644 --- a/backend/app/crud/__init__.py +++ b/backend/app/crud/__init__.py @@ -36,7 +36,6 @@ set_creds_for_org, get_creds_by_org, get_key_by_org, - get_provider_credential, update_creds_for_org, remove_creds_for_org, get_provider_credential, @@ -48,5 +47,5 @@ from .assistants import ( get_assistant_by_id, fetch_assistant_from_openai, - insert_assistant, + sync_assistant, ) diff --git a/backend/app/crud/assistants.py b/backend/app/crud/assistants.py index 6de2dd25..597664c7 100644 --- a/backend/app/crud/assistants.py +++ b/backend/app/crud/assistants.py @@ -36,14 +36,19 @@ def fetch_assistant_from_openai(assistant_id: str, client: OpenAI) -> OpenAIAssi try: assistant = client.beta.assistants.retrieve(assistant_id=assistant_id) return assistant + except openai.NotFoundError as e: + logger.error( + f"[fetch_assistant_from_openai] Assistant not found: {mask_string(assistant_id)} | {e}" + ) + raise HTTPException(status_code=404, detail="Assistant not found in OpenAI.") except openai.OpenAIError as e: logger.error( f"[fetch_assistant_from_openai] OpenAI API error while retrieving assistant {mask_string(assistant_id)}: {e}" ) - raise HTTPException(status_code=400, detail=f"OpenAI API error: {e}") + raise HTTPException(status_code=502, detail=f"OpenAI API error: {e}") -def insert_assistant( +def sync_assistant( session: Session, organization_id: int, project_id: int, diff --git a/backend/app/tests/api/routes/test_assistants.py b/backend/app/tests/api/routes/test_assistants.py index b3ca1d89..5f17f8ea 100644 --- a/backend/app/tests/api/routes/test_assistants.py +++ b/backend/app/tests/api/routes/test_assistants.py @@ -4,8 +4,6 @@ from app.main import app from app.tests.utils.openai import mock_openai_assistant -client = TestClient(app) - @pytest.fixture def normal_user_api_key_header(): @@ -15,12 +13,10 @@ def normal_user_api_key_header(): @patch("app.api.routes.assistants.fetch_assistant_from_openai") def test_ingest_assistant_success( mock_fetch_assistant, + client: TestClient, normal_user_api_key_header: str, ): """Test successful assistant ingestion from OpenAI.""" - - # Setup mock return value - mock_assistant = mock_openai_assistant() mock_fetch_assistant.return_value = mock_assistant @@ -34,27 +30,3 @@ 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.api.routes.assistants.configure_openai") -def test_ingest_assistant_openai_not_configured( - mock_configure_openai, - normal_user_api_key_header: dict, -): - """Test assistant ingestion failure when OpenAI is not configured.""" - - # Setup mock to return failure for OpenAI configuration - mock_configure_openai.return_value = (None, False) - - # Use a mock assistant ID - mock_assistant_id = "asst_123456789" - - response = client.post( - f"/api/v1/assistant/{mock_assistant_id}/ingest", - headers=normal_user_api_key_header, - ) - - assert response.status_code == 400 - response_json = response.json() - assert response_json["success"] is False - assert response_json["error"] == "OpenAI not configured for this organization." diff --git a/backend/app/tests/crud/test_assistants.py b/backend/app/tests/crud/test_assistants.py index 47457b84..585aa025 100644 --- a/backend/app/tests/crud/test_assistants.py +++ b/backend/app/tests/crud/test_assistants.py @@ -4,11 +4,11 @@ from app.tests.utils.openai import mock_openai_assistant from app.tests.utils.utils import get_project -from app.crud.assistants import insert_assistant +from app.crud.assistants import sync_assistant class TestAssistant: - def test_insert_assistant_success(self, db: Session): + def test_sync_assistant_success(self, db: Session): project = get_project(db) openai_assistant = mock_openai_assistant( assistant_id="asst_success", @@ -16,7 +16,7 @@ def test_insert_assistant_success(self, db: Session): max_num_results=20, ) - result = insert_assistant( + result = sync_assistant( db, project.organization_id, project.id, openai_assistant ) @@ -30,21 +30,21 @@ def test_insert_assistant_success(self, db: Session): assert result.temperature == openai_assistant.temperature assert result.max_num_results == 20 - def test_insert_assistant_already_exists(self, db: Session): + def test_sync_assistant_already_exists(self, db: Session): project = get_project(db) openai_assistant = mock_openai_assistant( assistant_id="asst_exists", ) - insert_assistant(db, project.organization_id, project.id, openai_assistant) + sync_assistant(db, project.organization_id, project.id, openai_assistant) with pytest.raises(HTTPException) as exc_info: - insert_assistant(db, project.organization_id, project.id, openai_assistant) + sync_assistant(db, project.organization_id, project.id, openai_assistant) assert exc_info.value.status_code == 409 assert "already exists" in exc_info.value.detail - def test_insert_assistant_no_instructions(self, db: Session): + def test_sync_assistant_no_instructions(self, db: Session): project = get_project(db) openai_assistant = mock_openai_assistant( assistant_id="asst_no_instructions", @@ -52,19 +52,19 @@ def test_insert_assistant_no_instructions(self, db: Session): openai_assistant.instructions = None with pytest.raises(HTTPException) as exc_info: - insert_assistant(db, project.organization_id, project.id, openai_assistant) + sync_assistant(db, project.organization_id, project.id, openai_assistant) assert exc_info.value.status_code == 400 assert "no instruction" in exc_info.value.detail - def test_insert_assistant_no_name(self, db: Session): + def test_sync_assistant_no_name(self, db: Session): project = get_project(db) openai_assistant = mock_openai_assistant( assistant_id="asst_no_name", ) openai_assistant.name = None - result = insert_assistant( + result = sync_assistant( db, project.organization_id, project.id, openai_assistant ) @@ -72,13 +72,13 @@ def test_insert_assistant_no_name(self, db: Session): assert result.assistant_id == openai_assistant.id assert result.project_id == project.id - def test_insert_assistant_no_vector_stores(self, db: Session): + def test_sync_assistant_no_vector_stores(self, db: Session): project = get_project(db) openai_assistant = mock_openai_assistant( assistant_id="asst_no_vectors", vector_store_ids=None ) - result = insert_assistant( + result = sync_assistant( db, project.organization_id, project.id, openai_assistant ) @@ -86,12 +86,12 @@ def test_insert_assistant_no_vector_stores(self, db: Session): assert result.assistant_id == openai_assistant.id assert result.project_id == project.id - def test_insert_assistant_no_tools(self, db: Session): + def test_sync_assistant_no_tools(self, db: Session): project = get_project(db) openai_assistant = mock_openai_assistant(assistant_id="asst_no_tools") openai_assistant.tool_resources = None - result = insert_assistant( + result = sync_assistant( db, project.organization_id, project.id, openai_assistant ) @@ -99,14 +99,14 @@ def test_insert_assistant_no_tools(self, db: Session): assert result.assistant_id == openai_assistant.id assert result.project_id == project.id - def test_insert_assistant_no_tool_resources(self, db: Session): + def test_sync_assistant_no_tool_resources(self, db: Session): project = get_project(db) openai_assistant = mock_openai_assistant( assistant_id="asst_no_tool_resources", ) openai_assistant.tools = None - result = insert_assistant( + result = sync_assistant( db, project.organization_id, project.id, openai_assistant ) diff --git a/backend/app/utils.py b/backend/app/utils.py index b7f7f793..8f5be811 100644 --- a/backend/app/utils.py +++ b/backend/app/utils.py @@ -5,16 +5,18 @@ from pathlib import Path from typing import Any, Dict, Generic, Optional, TypeVar -import emails # type: ignore import jwt +import emails from jinja2 import Template from jwt.exceptions import InvalidTokenError +from fastapi import HTTPException +from openai import OpenAI +from pydantic import BaseModel +from sqlmodel import Session from app.core import security from app.core.config import settings - -from typing import Generic, Optional, TypeVar -from pydantic import BaseModel +from app.crud.credentials import get_provider_credential logging.basicConfig(level=logging.INFO) logger = logging.getLogger(__name__) @@ -163,6 +165,32 @@ def mask_string(value: str, mask_char: str = "*") -> str: return value[:start] + (mask_char * num_mask) + value[end:] +def get_openai_client(session: Session, org_id: int, project_id: int) -> OpenAI: + """ + Fetch OpenAI credentials for the current org/project and return a configured client. + """ + credentials = get_provider_credential( + session=session, + org_id=org_id, + provider="openai", + project_id=project_id, + ) + + if not credentials or "api_key" not in credentials: + raise HTTPException( + status_code=400, + detail="OpenAI credentials not configured for this organization/project.", + ) + + try: + return OpenAI(api_key=credentials["api_key"]) + except Exception as e: + raise HTTPException( + status_code=500, + detail=f"Failed to configure OpenAI client: {str(e)}", + ) + + @ft.singledispatch def load_description(filename: Path) -> str: if not filename.exists(): From 091f78dae6b789ce0f0e8ed14a07fba28faae79f Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Wed, 16 Jul 2025 14:00:21 +0530 Subject: [PATCH 15/16] pre commit --- backend/app/crud/assistants.py | 4 ++-- backend/app/tests/api/routes/test_assistants.py | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/backend/app/crud/assistants.py b/backend/app/crud/assistants.py index 597664c7..4c5319f2 100644 --- a/backend/app/crud/assistants.py +++ b/backend/app/crud/assistants.py @@ -62,7 +62,7 @@ def sync_assistant( existing_assistant = get_assistant_by_id(session, assistant_id, organization_id) if existing_assistant: logger.info( - f"[insert_assistant] Assistant with ID {assistant_id} already exists in the database." + f"[sync_assistant] Assistant with ID {assistant_id} already exists in the database." ) raise HTTPException( status_code=409, @@ -108,6 +108,6 @@ def sync_assistant( session.refresh(db_assistant) logger.info( - f"[insert_assistant] Successfully ingested assistant with ID {mask_string(assistant_id)}." + f"[sync_assistant] Successfully ingested assistant with ID {mask_string(assistant_id)}." ) return db_assistant diff --git a/backend/app/tests/api/routes/test_assistants.py b/backend/app/tests/api/routes/test_assistants.py index 5f17f8ea..936e0c07 100644 --- a/backend/app/tests/api/routes/test_assistants.py +++ b/backend/app/tests/api/routes/test_assistants.py @@ -1,7 +1,6 @@ import pytest from fastapi.testclient import TestClient from unittest.mock import patch -from app.main import app from app.tests.utils.openai import mock_openai_assistant From 07cc058a4487b70d0c91424a3063a617b9003b1e Mon Sep 17 00:00:00 2001 From: Aviraj <100823015+avirajsingh7@users.noreply.github.com> Date: Wed, 16 Jul 2025 14:05:28 +0530 Subject: [PATCH 16/16] mask assistant id --- backend/app/crud/assistants.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/app/crud/assistants.py b/backend/app/crud/assistants.py index 4c5319f2..3f040508 100644 --- a/backend/app/crud/assistants.py +++ b/backend/app/crud/assistants.py @@ -62,7 +62,7 @@ def sync_assistant( existing_assistant = get_assistant_by_id(session, assistant_id, organization_id) if existing_assistant: logger.info( - f"[sync_assistant] Assistant with ID {assistant_id} already exists in the database." + f"[sync_assistant] Assistant with ID {mask_string(assistant_id)} already exists in the database." ) raise HTTPException( status_code=409,