Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ DOCKER_IMAGE_FRONTEND=frontend
AWS_ACCESS_KEY_ID=
AWS_SECRET_ACCESS_KEY=
AWS_DEFAULT_REGION=ap-south-1
AWS_S3_BUCKET_PREFIX = "bucket-prefix-name"
AWS_S3_BUCKET_PREFIX="bucket-prefix-name"

# OpenAI

Expand Down
47 changes: 37 additions & 10 deletions backend/app/api/routes/collections.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,13 @@
from app.api.deps import CurrentUser, SessionDep, CurrentUserOrgProject
from app.core.cloud import AmazonCloudStorage
from app.core.config import settings
from app.core.util import now, raise_from_unknown, post_callback
from app.crud import DocumentCrud, CollectionCrud, DocumentCollectionCrud
from app.core.util import now, raise_from_unknown, post_callback, configure_openai
Comment thread
nishika26 marked this conversation as resolved.
Outdated
from app.crud import (
DocumentCrud,
CollectionCrud,
DocumentCollectionCrud,
get_provider_credential,
)
from app.crud.rag import OpenAIVectorStoreCrud, OpenAIAssistantCrud
Comment thread
nishika26 marked this conversation as resolved.
from app.models import Collection, Document
from app.models.collection import CollectionStatus
Expand Down Expand Up @@ -178,14 +183,27 @@
)


def mark_collection_failed(session, user_id, collection_id, reason: str):
Comment thread
nishika26 marked this conversation as resolved.
Outdated
Comment thread
nishika26 marked this conversation as resolved.
Outdated
try:
collection = CollectionCrud(session, user_id).read_one(collection_id)
collection.status = CollectionStatus.failed
collection.updated_at = now()
CollectionCrud(session, user_id)._update(collection)
except Exception as suberr:
logger.warning(

Check warning on line 193 in backend/app/api/routes/collections.py

View check run for this annotation

Codecov / codecov/patch

backend/app/api/routes/collections.py#L187-L193

Added lines #L187 - L193 were not covered by tests
f"[do_create_collection] Failed to mark collection failed | {{'collection_id': '{collection_id}', 'reason': '{str(suberr)}'}}"
)

Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

def do_create_collection(
session: SessionDep,
current_user: CurrentUser,
current_user: CurrentUserOrgProject,
request: CreationRequest,
payload: ResponsePayload,
client,
Comment thread
nishika26 marked this conversation as resolved.
Outdated
):
start_time = time.time()
client = OpenAI(api_key=settings.OPENAI_API_KEY)

callback = (
SilentCallback(payload)
if request.callback_url is None
Expand Down Expand Up @@ -226,7 +244,7 @@
collection_crud._update(collection)

elapsed = time.time() - start_time
logging.info(
logger.info(
f"[do_create_collection] Collection created: {collection.id} | Time: {elapsed:.2f}s | "
f"Files: {len(flat_docs)} | Sizes: {file_sizes_kb} KB | Types: {list(file_exts)}"
)
Expand Down Expand Up @@ -261,6 +279,19 @@
request: CreationRequest,
background_tasks: BackgroundTasks,
):
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)
Comment thread
nishika26 marked this conversation as resolved.
Outdated
if not success:
logger.error(

Check warning on line 290 in backend/app/api/routes/collections.py

View check run for this annotation

Codecov / codecov/patch

backend/app/api/routes/collections.py#L290

Added line #L290 was not covered by tests
f"[create_collection] OpenAI API key not configured for org_id={current_user.organization_id}, project_id={current_user.project_id}"
)
raise HTTPException(status_code=400, detail="OpenAI is not configured")

Check warning on line 293 in backend/app/api/routes/collections.py

View check run for this annotation

Codecov / codecov/patch

backend/app/api/routes/collections.py#L293

Added line #L293 was not covered by tests

this = inspect.currentframe()
route = router.url_path_for(this.f_code.co_name)
payload = ResponsePayload("processing", route)
Expand All @@ -278,11 +309,7 @@

# 2. Launch background task
background_tasks.add_task(
do_create_collection,
session,
current_user,
request,
payload,
do_create_collection, session, current_user, request, payload, client
)

logger.info(
Expand Down
41 changes: 34 additions & 7 deletions backend/app/api/routes/documents.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
from typing import List
from pathlib import Path

from fastapi import APIRouter, File, UploadFile, Query
from fastapi import APIRouter, File, UploadFile, Query, HTTPException
from fastapi import Path as FastPath

from app.crud import DocumentCrud, CollectionCrud
from app.crud import DocumentCrud, CollectionCrud, get_provider_credential
from app.models import Document
from app.core.util import configure_openai
Comment thread
nishika26 marked this conversation as resolved.
Outdated
from app.utils import APIResponse, load_description
from app.api.deps import CurrentUser, SessionDep
from app.api.deps import CurrentUser, SessionDep, CurrentUserOrgProject
from app.core.cloud import AmazonCloudStorage
from app.crud.rag import OpenAIAssistantCrud

Expand Down Expand Up @@ -65,10 +66,23 @@
)
def remove_doc(
session: SessionDep,
current_user: CurrentUser,
current_user: CurrentUserOrgProject,
doc_id: UUID = FastPath(description="Document to delete"),
):
a_crud = OpenAIAssistantCrud()
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)
Comment thread
nishika26 marked this conversation as resolved.
Outdated
if not success:
logger.error(

Check warning on line 80 in backend/app/api/routes/documents.py

View check run for this annotation

Codecov / codecov/patch

backend/app/api/routes/documents.py#L80

Added line #L80 was not covered by tests
f"[remove_doc] OpenAI API key not configured for org_id={current_user.organization_id}, project_id={current_user.project_id}"
)
raise HTTPException(status_code=400, detail="OpenAI is not configured")

Check warning on line 83 in backend/app/api/routes/documents.py

View check run for this annotation

Codecov / codecov/patch

backend/app/api/routes/documents.py#L83

Added line #L83 was not covered by tests

a_crud = OpenAIAssistantCrud(client)
d_crud = DocumentCrud(session, current_user.id)
c_crud = CollectionCrud(session, current_user.id)

Expand All @@ -84,10 +98,23 @@
)
def permanent_delete_doc(
session: SessionDep,
current_user: CurrentUser,
current_user: CurrentUserOrgProject,
doc_id: UUID = FastPath(description="Document to permanently delete"),
):
a_crud = OpenAIAssistantCrud()
credentials = get_provider_credential(
session=session,
org_id=current_user.organization_id,
provider="openai",
project_id=current_user.project_id,
)
Comment thread
nishika26 marked this conversation as resolved.
client, success = configure_openai(credentials)
Comment thread
nishika26 marked this conversation as resolved.
Outdated
if not success:
logger.error(
f"[permanent_delete_doc] OpenAI API key not configured for org_id={current_user.organization_id}, project_id={current_user.project_id}"
)
raise HTTPException(status_code=400, detail="OpenAI is not configured")

a_crud = OpenAIAssistantCrud(client)
d_crud = DocumentCrud(session, current_user.id)
c_crud = CollectionCrud(session, current_user.id)
storage = AmazonCloudStorage(current_user)
Expand Down
1 change: 1 addition & 0 deletions backend/app/crud/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
get_key_by_org,
update_creds_for_org,
remove_creds_for_org,
get_provider_credential,
)

from .thread_results import upsert_thread_result, get_thread_result
Expand Down
6 changes: 4 additions & 2 deletions backend/app/crud/rag/open_ai.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,10 @@


class OpenAICrud:
def __init__(self, client=None):
self.client = client or OpenAI(api_key=settings.OPENAI_API_KEY)
def __init__(self, client):
Comment thread
nishika26 marked this conversation as resolved.
if client is None:
raise ValueError("OpenAI client is not configured")

Check warning on line 95 in backend/app/crud/rag/open_ai.py

View check run for this annotation

Codecov / codecov/patch

backend/app/crud/rag/open_ai.py#L95

Added line #L95 was not covered by tests
Comment thread
avirajsingh7 marked this conversation as resolved.
self.client = client


class OpenAIVectorStoreCrud(OpenAICrud):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,18 @@
import io

import openai_responses
from sqlmodel import Session, select
from sqlmodel import Session
from fastapi.testclient import TestClient
from openai import OpenAIError
from unittest.mock import MagicMock, patch

from app.core.config import settings
from app.tests.utils.document import DocumentStore
from app.tests.utils.utils import openai_credentials, get_user_from_api_key
from app.tests.utils.utils import get_user_from_api_key
from app.main import app
from app.crud.collection import CollectionCrud
from app.api.routes.collections import CreationRequest, ResponsePayload
from app.seed_data.seed_data import seed_database
from app.models.collection import CollectionStatus
from app.tests.utils.collections_openai_mock import get_mock_openai_client

client = TestClient(app)

Expand Down Expand Up @@ -51,17 +51,21 @@ def head_object(self, Bucket, Key):
monkeypatch.setattr("boto3.client", lambda service: FakeS3Client())


@pytest.mark.usefixtures("openai_credentials")
@patch("app.api.routes.collections.configure_openai")
@patch("app.api.routes.collections.get_provider_credential")
class TestCollectionRouteCreate:
_n_documents = 5

@openai_responses.mock()
def test_create_collection_success(
self,
mock_get_credential,
mock_configure_openai,
client: TestClient,
Comment thread
nishika26 marked this conversation as resolved.
db: Session,
api_key_headers: dict[str, str],
):
store = DocumentStore(db)
# Setup test documents
store = DocumentStore(db, api_key_headers)
documents = store.fill(self._n_documents)
doc_ids = [str(doc.id) for doc in documents]

Expand All @@ -73,9 +77,13 @@ def test_create_collection_success(
"temperature": 0.1,
}
original_api_key = "ApiKey No3x47A5qoIGhm0kVKjQ77dhCqEdWRIQZlEPzzzh7i8"

headers = {"X-API-KEY": original_api_key}

mock_get_credential.return_value = {"api_key": "test_api_key"}

mock_openai_client = get_mock_openai_client()
mock_configure_openai.return_value = (mock_openai_client, True)

Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
response = client.post(
f"{settings.API_V1_STR}/collections/create",
json=body,
Expand All @@ -89,8 +97,8 @@ def test_create_collection_success(
assert metadata["status"] == CollectionStatus.processing.value
assert UUID(metadata["key"])

# Confirm collection metadata in DB
collection_id = UUID(metadata["key"])

user = get_user_from_api_key(db, headers)
collection = CollectionCrud(db, user.user_id).read_one(collection_id)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ def test_response_is_success(
db: Session,
route: Route,
crawler: WebCrawler,
api_key_headers: dict[str, str],
):
store = DocumentStore(db)
store = DocumentStore(db, api_key_headers)
response = crawler.get(route.append(store.put()))

assert response.is_success
Expand All @@ -34,8 +35,9 @@ def test_info_reflects_database(
db: Session,
route: Route,
crawler: WebCrawler,
api_key_headers: dict[str, str],
):
store = DocumentStore(db)
store = DocumentStore(db, api_key_headers)
document = store.put()
source = DocumentComparator(document)

Expand All @@ -44,13 +46,10 @@ def test_info_reflects_database(
assert source == target.data

def test_cannot_info_unknown_document(
self,
db: Session,
route: Route,
crawler: Route,
self, db: Session, route: Route, crawler: Route, api_key_headers: dict[str, str]
):
DocumentStore.clear(db)
maker = DocumentMaker(db)
maker = DocumentMaker(db, api_key_headers)
response = crawler.get(route.append(next(maker)))

assert response.is_error
Comment on lines 48 to 53
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix the type annotation for the crawler parameter.

The crawler parameter should be of type WebCrawler, not Route, based on how it's used in the test and consistency with other test methods.

Apply this diff to fix the type annotation:

 def test_cannot_info_unknown_document(
-    self, db: Session, route: Route, crawler: Route, api_key_headers: dict[str, str]
+    self, db: Session, route: Route, crawler: WebCrawler, api_key_headers: dict[str, str]
 ):
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def test_cannot_info_unknown_document(
self,
db: Session,
route: Route,
crawler: Route,
self, db: Session, route: Route, crawler: Route, api_key_headers: dict[str, str]
):
DocumentStore.clear(db)
maker = DocumentMaker(db)
maker = DocumentMaker(db, api_key_headers)
response = crawler.get(route.append(next(maker)))
assert response.is_error
def test_cannot_info_unknown_document(
self, db: Session, route: Route, crawler: WebCrawler, api_key_headers: dict[str, str]
):
🧰 Tools
🪛 Ruff (0.12.2)

49-49: Redefinition of unused crawler from line 10

(F811)

🤖 Prompt for AI Agents
In backend/app/tests/api/routes/documents/test_route_document_info.py around
lines 48 to 55, the type annotation for the crawler parameter is incorrectly set
as Route. Change the type annotation of crawler from Route to WebCrawler to
match its actual usage and maintain consistency with other test methods.

Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,9 @@ def test_item_reflects_database(
db: Session,
route: QueryRoute,
crawler: WebCrawler,
api_key_headers: dict[str, str],
):
store = DocumentStore(db)
store = DocumentStore(db, api_key_headers)
source = DocumentComparator(store.put())

response = httpx_to_standard(crawler.get(route))
Expand Down Expand Up @@ -77,8 +78,9 @@ def test_skip_greater_than_limit_is_difference(
db: Session,
route: QueryRoute,
crawler: WebCrawler,
api_key_headers: dict[str, str],
):
store = DocumentStore(db)
store = DocumentStore(db, api_key_headers)
limit = len(store.fill(self._ndocs))
skip = limit // 2

Expand Down
Loading