refactor(api): Unify chat endpoint and process file uploads concurrently

chatbot-core/api/routes/chatbot.py

@@ -34,7 +34,6 @@
 # Local application imports
 # =========================
 from api.models.schemas import (
-    ChatRequest,
     ChatResponse,
     DeleteResponse,
     MessageHistoryResponse,
@@ -225,103 +224,95 @@ def get_chat_history(session_id: str):
     )
 
 
-# Chat Endpoint
-@router.post("/sessions/{session_id}/message", response_model=ChatResponse)
-def chatbot_reply(session_id: str, request: ChatRequest, _background_tasks: BackgroundTasks):
-
+async def _process_one_file(upload_file: UploadFile) -> FileAttachment:
     """
-    POST endpoint to handle chatbot replies.
-
-    Receives a user message and returns the assistant's reply.
-    Validates that the session exists before processing.
-
-    Args:
-        session_id (str): The session identifier.
-        request (ChatRequest): The request containing the user message.
+    Asynchronously read and process a single uploaded file.
 
-    Returns:
-        ChatResponse: The assistant's reply.
+    Ensures the file is closed after processing. Runs the potentially
+    blocking `process_uploaded_file` in a separate thread.
     """
-    if not session_exists(session_id):
-        raise HTTPException(
-            status_code=404,
-            detail="Session not found.",
-        )
-    reply =  get_chatbot_reply(session_id, request.message)
-    _background_tasks.add_task(
-        persist_session,
-        session_id,
+    try:
+        content = await upload_file.read()
+        processed = await asyncio.to_thread(
+            process_uploaded_file, content, upload_file.filename or "unknown"
         )
-
-    return reply
+        return FileAttachment(**processed)
+    finally:
+        await upload_file.close()
 
 
 @router.post(
-    "/sessions/{session_id}/message/upload",
+    "/sessions/{session_id}/message",
     response_model=ChatResponse,
 )
-async def chatbot_reply_with_files(
+async def chatbot_reply(
     session_id: str,
     background_tasks: BackgroundTasks,
-    message: str = Form(...),
+    message: Optional[str] = Form(None),
     files: Optional[List[UploadFile]] = File(None),
 ):
     """
-    POST endpoint to handle chatbot replies with file uploads.
+    POST endpoint to handle chatbot replies, with optional file uploads.
 
     Receives a user message with optional file attachments and returns
-    the assistant's reply. Files are processed and their content is
-    included in the context for the LLM.
+    the assistant's reply. This endpoint handles both standard messages
+    and messages with files using multipart/form-data.
+
+    If only files are provided, a default message will be used.
 
     Supported file types:
     - Text files: .txt, .log, .md, .json, .xml, .yaml, .yml, code files
     - Image files: .png, .jpg, .jpeg, .gif, .webp, .bmp
 
     Args:
         session_id (str): The ID of the session from the URL path.
-        message (str): The user's message (form field).
-        files (List[UploadFile]): Optional list of uploaded files.
+        background_tasks (BackgroundTasks): FastAPI background tasks.
+        message (Optional[str]): The user's message (form field).
+        files (Optional[List[UploadFile]]): Optional list of uploaded files.
 
     Returns:
         ChatResponse: The chatbot's generated reply.
 
     Raises:
         HTTPException: 404 if session not found, 400 if file processing fails,
-                      422 if message is empty and no files provided.
+                      422 if neither message nor files are provided.
     """
     if not session_exists(session_id):
         raise HTTPException(status_code=404, detail="Session not found.")
 
     # Validate that at least message or files are provided
     has_message = message and message.strip()
-    has_files = files and len(files) > 0
+    has_files = files is not None and len(files) > 0
 
     if not has_message and not has_files:
         raise HTTPException(
             status_code=422,
-            detail="Either message or files must be provided.",
+            detail="Either a message or at least one file must be provided.",
         )
 
     # Process uploaded files
     processed_files: List[FileAttachment] = []
 
     if files:
-        for upload_file in files:
-            try:
-                content = await upload_file.read()
-                processed = process_uploaded_file(
-                    content, upload_file.filename or "unknown"
+        tasks = [_process_one_file(f) for f in files]
+        results = await asyncio.gather(*tasks, return_exceptions=True)
+
+        for result in results:
+            if isinstance(result, Exception):
+                if isinstance(result, FileProcessingError):
+                    raise HTTPException(status_code=400, detail=str(result)) from result
+
+                logger.error(
+                    "Unexpected error processing file for session %s: %s",
+                    session_id,
+                    result,
+                    exc_info=True,
                 )
-                processed_files.append(FileAttachment(**processed))
-            except FileProcessingError as e:
-                raise HTTPException(status_code=400, detail=str(e)) from e
-            except Exception as e:
                 raise HTTPException(
                     status_code=500,
-                    detail=f"Failed to process file: {type(e).__name__}",
-                ) from e
-            finally:
-                await upload_file.close()
+                    detail=f"Failed to process file: {type(result).__name__}",
+                ) from result
+            processed_files.append(result)
 
     # Use default message if only files provided
     final_message = (

chatbot-core/tests/integration/test_chatbot.py

@@ -29,7 +29,7 @@ def test_reply_to_existing_session(client, mock_llm_provider, mock_get_relevant_
     mock_get_relevant_documents.return_value = get_relevant_documents_output()
 
     payload = {"message": "Hello"}
-    response = client.post(f"/sessions/{session_id}/message", json=payload)
+    response = client.post(f"/sessions/{session_id}/message", data=payload)
 
     assert response.status_code == 200
     try:
@@ -41,7 +41,7 @@ def test_reply_to_existing_session(client, mock_llm_provider, mock_get_relevant_
 def test_reply_to_nonexistent_session(client):
     """Should return 404 when replying to a non-existent session."""
     payload = {"message": "Hello"}
-    response = client.post("/sessions/nonexistent-session/message", json=payload)
+    response = client.post("/sessions/nonexistent-session/message", data=payload)
 
     assert response.status_code == 404
     assert response.json() == {"detail": "Session not found."}
@@ -72,7 +72,7 @@ def test_reply_after_session_deleted(client):
     client.delete(f"/sessions/{session_id}")
 
     payload = {"message": "Is anyone there?"}
-    response = client.post(f"/sessions/{session_id}/message", json=payload)
+    response = client.post(f"/sessions/{session_id}/message", data=payload)
 
     assert response.status_code == 404
     assert response.json() == {"detail": "Session not found."}
@@ -84,11 +84,10 @@ def test_reply_with_empty_message(client):
     session_id = create_resp.json()["session_id"]
 
     payload = {"message": "   "}
-    response = client.post(f"/sessions/{session_id}/message", json=payload)
+    response = client.post(f"/sessions/{session_id}/message", data=payload)
 
     assert response.status_code == 422
-    errors = response.json()["detail"]
-    assert any("Message cannot be empty." in e["msg"] for e in errors)
+    assert response.json()["detail"] == "Either a message or at least one file must be provided."
 
 
 def test_full_chat_lifecycle(client, mock_llm_provider, mock_get_relevant_documents):
@@ -101,7 +100,7 @@ def test_full_chat_lifecycle(client, mock_llm_provider, mock_get_relevant_docume
     session_id = create_resp.json()["session_id"]
 
     payload = {"message": "Hello"}
-    reply_resp = client.post(f"/sessions/{session_id}/message", json=payload)
+    reply_resp = client.post(f"/sessions/{session_id}/message", data=payload)
     assert reply_resp.status_code == 200
     assert reply_resp.json()["reply"] == "Hello from the bot!"
 
@@ -122,7 +121,7 @@ def test_multiple_messages_in_session(client, mock_llm_provider, mock_get_releva
     ]
     session_id = client.post("/sessions").json()["session_id"]
     for i in range(3):
-        resp = client.post(f"/sessions/{session_id}/message", json={"message": f"Msg {i+1}"})
+        resp = client.post(f"/sessions/{session_id}/message", data={"message": f"Msg {i+1}"})
         assert resp.status_code == 200
         assert resp.json()["reply"] == f"Reply {i+1}"
 
@@ -135,14 +134,14 @@ def test_multiple_sessions_are_isolated(client, mock_llm_provider, mock_get_rele
     active_session = client.post("/sessions").json()["session_id"]
     deleted_session = client.post("/sessions").json()["session_id"]
 
-    client.post(f"/sessions/{active_session}/message", json={"message": "Hi A"})
-    client.post(f"/sessions/{deleted_session}/message", json={"message": "Hi B"})
+    client.post(f"/sessions/{active_session}/message", data={"message": "Hi A"})
+    client.post(f"/sessions/{deleted_session}/message", data={"message": "Hi B"})
 
     client.delete(f"/sessions/{deleted_session}")
     response_active_session = client.post(f"/sessions/{active_session}/message",
-                                json={"message": "Message again"})
+                                data={"message": "Message again"})
     response_deleted_session = client.post(f"/sessions/{deleted_session}/message",
-                                json={"message": "Should be off"})
+                                data={"message": "Should be off"})
 
     assert response_active_session.status_code == 200
     assert response_deleted_session.status_code == 404
@@ -166,7 +165,7 @@ def test_get_history_with_messages(client, mock_llm_provider, mock_get_relevant_
     mock_get_relevant_documents.return_value = get_relevant_documents_output()
 
     session_id = client.post("/sessions").json()["session_id"]
-    client.post(f"/sessions/{session_id}/message", json={"message": "Hello"})
+    client.post(f"/sessions/{session_id}/message", data={"message": "Hello"})
 
     response = client.get(f"/sessions/{session_id}/message")
     assert response.status_code == 200

chatbot-core/tests/unit/routes/test_chatbot.py

@@ -17,8 +17,7 @@ def test_chatbot_reply_success(client, mock_session_exists, mock_get_chatbot_rep
     mock_get_chatbot_reply.return_value = {"reply": "This is a valid response"}
     data = {"message": "This is a valid query"}
 
-    response = client.post("/sessions/test-session-id/message", json=data)
-
+    response = client.post("/sessions/test-session-id/message", data=data)
     assert response.status_code == 200
     assert response.json() == {"reply": "This is a valid response"}
 
@@ -28,7 +27,8 @@ def test_chatbot_reply_invalid_session(client, mock_session_exists):
     mock_session_exists.return_value = False
     data = {"message": "This is a valid query"}
 
-    response = client.post("/sessions/invalid-session-id/message", json=data)
+
+    response = client.post("/sessions/invalid-session-id/message", data=data)
 
     assert response.status_code == 404
     assert response.json() == {"detail": "Session not found."}
@@ -38,12 +38,11 @@ def test_chatbot_reply_empty_message_returns_422(client, mock_session_exists):
     """Testing that if sending an empty message returns 422 validation error."""
     mock_session_exists.return_value = True
     data = {"message": "   "}
-    response = client.post("/sessions/test-session-id/message", json=data)
-
-    errors = response.json()["detail"]
 
+    response = client.post("/sessions/test-session-id/message", data=data)
+    detail = response.json()["detail"]
     assert response.status_code == 422
-    assert "Message cannot be empty." in errors[0]["msg"]
+    assert detail == "Either a message or at least one file must be provided."
 
 
 def test_delete_chat_success(client, mock_delete_session):